2.6.32-longterm review patch.  If anyone has any objections, please let me know.

------------------

From: Jann Horn <j...@thejh.net>

commit 8b01fc86b9f425899f8a3a8fc1c47d73c2c20543 upstream.

This prevents a race between chown() and execve(), where chowning a
setuid-user binary to root would momentarily make the binary setuid
root.

This patch was mostly written by Linus Torvalds.

Signed-off-by: Jann Horn <j...@thejh.net>
Signed-off-by: Linus Torvalds <torva...@linux-foundation.org>
[bwh: Backported to 3.2:
 - Drop the task_no_new_privs() and user namespace checks
 - Open-code file_inode()
 - s/READ_ONCE/ACCESS_ONCE/
 - Adjust context]
Signed-off-by: Ben Hutchings <b...@decadent.org.uk>
(cherry picked from commit 470e517be17dd6ef8670bec7bd7831ea0d3ad8a6)

Signed-off-by: Willy Tarreau <w...@1wt.eu>
---
 fs/exec.c | 65 +++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index c32ae34..8dc1270 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1181,6 +1181,45 @@ int check_unsafe_exec(struct linux_binprm *bprm)
        return res;
 }
 
+static void bprm_fill_uid(struct linux_binprm *bprm)
+{
+       struct inode *inode;
+       unsigned int mode;
+       uid_t uid;
+       gid_t gid;
+
+       /* clear any previous set[ug]id data from a previous binary */
+       bprm->cred->euid = current_euid();
+       bprm->cred->egid = current_egid();
+
+       if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+               return;
+
+       inode = bprm->file->f_path.dentry->d_inode;
+       mode = ACCESS_ONCE(inode->i_mode);
+       if (!(mode & (S_ISUID|S_ISGID)))
+               return;
+
+       /* Be careful if suid/sgid is set */
+       mutex_lock(&inode->i_mutex);
+
+       /* reload atomically mode/uid/gid now that lock held */
+       mode = inode->i_mode;
+       uid = inode->i_uid;
+       gid = inode->i_gid;
+       mutex_unlock(&inode->i_mutex);
+
+       if (mode & S_ISUID) {
+               bprm->per_clear |= PER_CLEAR_ON_SETID;
+               bprm->cred->euid = uid;
+       }
+
+       if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
+               bprm->per_clear |= PER_CLEAR_ON_SETID;
+               bprm->cred->egid = gid;
+       }
+}
+
 /* 
  * Fill the binprm structure from the inode. 
  * Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
@@ -1189,36 +1228,12 @@ int check_unsafe_exec(struct linux_binprm *bprm)
  */
 int prepare_binprm(struct linux_binprm *bprm)
 {
-       umode_t mode;
-       struct inode * inode = bprm->file->f_path.dentry->d_inode;
        int retval;
 
-       mode = inode->i_mode;
        if (bprm->file->f_op == NULL)
                return -EACCES;
 
-       /* clear any previous set[ug]id data from a previous binary */
-       bprm->cred->euid = current_euid();
-       bprm->cred->egid = current_egid();
-
-       if (!(bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)) {
-               /* Set-uid? */
-               if (mode & S_ISUID) {
-                       bprm->per_clear |= PER_CLEAR_ON_SETID;
-                       bprm->cred->euid = inode->i_uid;
-               }
-
-               /* Set-gid? */
-               /*
-                * If setgid is set but no group execute bit then this
-                * is a candidate for mandatory locking, not a setgid
-                * executable.
-                */
-               if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
-                       bprm->per_clear |= PER_CLEAR_ON_SETID;
-                       bprm->cred->egid = inode->i_gid;
-               }
-       }
+       bprm_fill_uid(bprm);
 
        /* fill in binprm security blob */
        retval = security_bprm_set_creds(bprm);
-- 
1.7.12.2.21.g234cd45.dirty



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to