Commit ccfd4cc5 authored by Chris Wright's avatar Chris Wright Committed by Linus Torvalds

[PATCH] lsm: setfsuid/setgsuid bug fix (4/4)

Patch from Jakub Jelínek <jakub@redhat.com>

Make sure setfsuid/setfsgid return values are right.  Before
include/linux/security.h was added, setfsuid/setfsgid always returned
old_fsuid, no matter if the fsuid was actually changed or not.

With the default security ops it seems to do the same, because both
security_task_setuid and security_task_post_setuid return 0, but these
are hooks which seem to return 0 on success, -errno on failure, so if
some non-default security hook is installed and ever returns -errno in
setfsuid/setfsgid, -errno will be returned from the syscall instead of
the expected old_fsuid.  This makes it hard to distinguish uids
0xfffff001 ..  0xffffffff from errors of security hooks.
parent c82a77d7
...@@ -831,13 +831,11 @@ asmlinkage long sys_getresgid(gid_t *rgid, gid_t *egid, gid_t *sgid) ...@@ -831,13 +831,11 @@ asmlinkage long sys_getresgid(gid_t *rgid, gid_t *egid, gid_t *sgid)
asmlinkage long sys_setfsuid(uid_t uid) asmlinkage long sys_setfsuid(uid_t uid)
{ {
int old_fsuid; int old_fsuid;
int retval;
retval = security_task_setuid(uid, (uid_t)-1, (uid_t)-1, LSM_SETID_FS);
if (retval)
return retval;
old_fsuid = current->fsuid; old_fsuid = current->fsuid;
if (security_task_setuid(uid, (uid_t)-1, (uid_t)-1, LSM_SETID_FS))
return old_fsuid;
if (uid == current->uid || uid == current->euid || if (uid == current->uid || uid == current->euid ||
uid == current->suid || uid == current->fsuid || uid == current->suid || uid == current->fsuid ||
capable(CAP_SETUID)) capable(CAP_SETUID))
...@@ -850,9 +848,7 @@ asmlinkage long sys_setfsuid(uid_t uid) ...@@ -850,9 +848,7 @@ asmlinkage long sys_setfsuid(uid_t uid)
current->fsuid = uid; current->fsuid = uid;
} }
retval = security_task_post_setuid(old_fsuid, (uid_t)-1, (uid_t)-1, LSM_SETID_FS); security_task_post_setuid(old_fsuid, (uid_t)-1, (uid_t)-1, LSM_SETID_FS);
if (retval)
return retval;
return old_fsuid; return old_fsuid;
} }
...@@ -863,13 +859,11 @@ asmlinkage long sys_setfsuid(uid_t uid) ...@@ -863,13 +859,11 @@ asmlinkage long sys_setfsuid(uid_t uid)
asmlinkage long sys_setfsgid(gid_t gid) asmlinkage long sys_setfsgid(gid_t gid)
{ {
int old_fsgid; int old_fsgid;
int retval;
retval = security_task_setgid(gid, (gid_t)-1, (gid_t)-1, LSM_SETID_FS);
if (retval)
return retval;
old_fsgid = current->fsgid; old_fsgid = current->fsgid;
if (security_task_setgid(gid, (gid_t)-1, (gid_t)-1, LSM_SETID_FS))
return old_fsgid;
if (gid == current->gid || gid == current->egid || if (gid == current->gid || gid == current->egid ||
gid == current->sgid || gid == current->fsgid || gid == current->sgid || gid == current->fsgid ||
capable(CAP_SETGID)) capable(CAP_SETGID))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment