summaryrefslogtreecommitdiff
path: root/fs/exec.c
diff options
context:
space:
mode:
authorEric W. Biederman <ebiederm@xmission.com>2020-05-29 08:24:10 -0500
committerEric W. Biederman <ebiederm@xmission.com>2020-05-29 21:06:48 -0500
commita7868323c2638a7c6c5b30b37831b73cbdf0dc15 (patch)
tree645cd8ae2bb31c07ebb2831b692aa6615cf6fc25 /fs/exec.c
parente32f8879019535b899bc3d51f371e17526f208d1 (diff)
exec: Add a per bprm->file version of per_clear
There is a small bug in the code that recomputes parts of bprm->cred for every bprm->file. The code never recomputes the part of clear_dangerous_personality_flags it is responsible for. Which means that in practice if someone creates a sgid script the interpreter will not be able to use any of: READ_IMPLIES_EXEC ADDR_NO_RANDOMIZE ADDR_COMPAT_LAYOUT MMAP_PAGE_ZERO. This accentially clearing of personality flags probably does not matter in practice because no one has complained but it does make the code more difficult to understand. Further remaining bug compatible prevents the recomputation from being removed and replaced by simply computing bprm->cred once from the final bprm->file. Making this change removes the last behavior difference between computing bprm->creds from the final file and recomputing bprm->cred several times. Which allows this behavior change to be justified for it's own reasons, and for any but hunts looking into why the behavior changed to wind up here instead of in the code that will follow that computes bprm->cred from the final bprm->file. This small logic bug appears to have existed since the code started clearing dangerous personality bits. History Tree: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Fixes: 1bb0fa189c6a ("[PATCH] NX: clean up legacy binary support") Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Diffstat (limited to 'fs/exec.c')
-rw-r--r--fs/exec.c6
1 files changed, 4 insertions, 2 deletions
diff --git a/fs/exec.c b/fs/exec.c
index c3c879a55d65..0f793536e393 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1354,6 +1354,7 @@ int begin_new_exec(struct linux_binprm * bprm)
me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD |
PF_NOFREEZE | PF_NO_SETAFFINITY);
flush_thread();
+ bprm->per_clear |= bprm->pf_per_clear;
me->personality &= ~bprm->per_clear;
/*
@@ -1628,12 +1629,12 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
return;
if (mode & S_ISUID) {
- bprm->per_clear |= PER_CLEAR_ON_SETID;
+ bprm->pf_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->pf_per_clear |= PER_CLEAR_ON_SETID;
bprm->cred->egid = gid;
}
}
@@ -1654,6 +1655,7 @@ static int prepare_binprm(struct linux_binprm *bprm)
/* Recompute parts of bprm->cred based on bprm->file */
bprm->active_secureexec = 0;
+ bprm->pf_per_clear = 0;
bprm_fill_uid(bprm);
retval = security_bprm_repopulate_creds(bprm);
if (retval)