userns: unshare_userns(&cred) should not populate cred on failure
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 userns: unshare_userns(&cred) should not populate cred on failure
unshare_userns(new_cred) does *new_cred = prepare_creds() before create_user_ns() which can fail. However, the caller expects that it doesn't need to take care of new_cred if unshare_userns() fails.
We could change the single caller, sys_unshare(), but I think it would be more clean to avoid the side effects on failure, so with this patch unshare_userns() does put_cred() itself and initializes *new_cred only if create_user_ns() succeeeds.
--- kernel/user_namespace.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)
kernel/user_namespace.c +9 -4
@@ -105,16 +105,21 @@ int create_user_ns(struct cred *new)int unshare_userns (unsigned long unshare_flags,struct cred **new_cred) {struct cred *cred; +int err = -ENOMEM ;if (!(unshare_flags &CLONE_NEWUSER ))return 0 ; cred =prepare_creds (); -if (!cred) -return -ENOMEM ; +if (cred) { + err =create_user_ns (cred); +if (err) +put_cred (cred); +else + *new_cred = cred; + } - *new_cred = cred; -return create_user_ns (cred); +return err; }void free_user_ns (struct user_namespace *ns)
Signature · 1 line
1.5.5.1
Quoted 46 lines · Oleg Nesterov
unshare_userns(new_cred)does*new_cred=prepare_creds()beforecreate_user_ns()which can fail. However, the caller expects that it doesn't need to take care ofnew_credifunshare_userns()fails.We could change the single caller,
sys_unshare(), but I think it would be more clean to avoid the side effects on failure, so with this patchunshare_userns()doesput_cred()itself and initializes*new_credonly ifcreate_user_ns()succeeeds.Signed-off-by Oleg Nesterov <[email protected]>--- kernel/user_namespace.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)kernel/user_namespace.c +9
-5@@ -105,16 +105,21 @@ int create_user_ns(struct cred *new)int unshare_userns (unsigned long unshare_flags,struct cred **new_cred) {struct cred *cred; +int err = -ENOMEM ;if (!(unshare_flags &CLONE_NEWUSER ))return 0 ; cred =prepare_creds (); -if (!cred) -return -ENOMEM ; +if (cred) { + err =create_user_ns (cred); +if (err) +put_cred (cred); +else + *new_cred = cred; + } - *new_cred = cred; -return create_user_ns (cred); +return err; }void free_user_ns (struct user_namespace *ns) --
Quoted 3 lines
1.5.5.1
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Quoted 8 lines · Oleg Nesterov
unshare_userns(new_cred)does*new_cred=prepare_creds()beforecreate_user_ns()which can fail. However, the caller expects that it doesn't need to take care ofnew_credifunshare_userns()fails.We could change the single caller,
sys_unshare(), but I think it would be more clean to avoid the side effects on failure, so with this patchunshare_userns()doesput_cred()itself and initializes*new_credonly ifcreate_user_ns()succeeeds.
Doh!
Quoted 36 lines
Signed-off-by Oleg Nesterov <[email protected]>--- kernel/user_namespace.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)kernel/user_namespace.c +9
-4@@ -105,16 +105,21 @@ int create_user_ns(struct cred *new)int unshare_userns (unsigned long unshare_flags,struct cred **new_cred) {struct cred *cred; +int err = -ENOMEM ;if (!(unshare_flags &CLONE_NEWUSER ))return 0 ; cred =prepare_creds (); -if (!cred) -return -ENOMEM ; +if (cred) { + err =create_user_ns (cred); +if (err) +put_cred (cred); +else + *new_cred = cred; + } - *new_cred = cred; -return create_user_ns (cred); +return err; }void free_user_ns (struct user_namespace *ns)