@@ -1590,21 +1590,23 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
static void bprm_fill_uid(struct linux_binprm *bprm)
{
/* Handle suid and sgid on files */
+ struct cred *new = bprm->cred;
struct inode *inode;
unsigned int mode;
+ bool need_cap;
kuid_t uid;
kgid_t gid;
if (!mnt_may_suid(bprm->file->f_path.mnt))
- return;
+ goto after_setid;
if (task_no_new_privs(current))
- return;
+ goto after_setid;
inode = bprm->file->f_path.dentry->d_inode;
mode = READ_ONCE(inode->i_mode);
if (!(mode & (S_ISUID|S_ISGID)))
- return;
+ goto after_setid;
/* Be careful if suid/sgid is set */
inode_lock(inode);
@@ -1616,19 +1618,50 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
inode_unlock(inode);
/* We ignore suid/sgid if there are no mappings for them in the ns */
- if (!kuid_has_mapping(bprm->cred->user_ns, uid) ||
- !kgid_has_mapping(bprm->cred->user_ns, gid))
- return;
+ if (!kuid_has_mapping(new->user_ns, uid) ||
+ !kgid_has_mapping(new->user_ns, gid))
+ goto after_setid;
if (mode & S_ISUID) {
bprm->per_clear = 1;
- bprm->cred->euid = uid;
+ new->euid = uid;
}
if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
bprm->per_clear = 1;
- bprm->cred->egid = gid;
+ new->egid = gid;
+ }
+
+after_setid:
+ /* Will the new creds have multiple uids or gids? */
+ if (!uid_eq(new->euid, new->uid) || !gid_eq(new->egid, new->gid)) {
+ bprm->secureexec = 1;
+
+ /*
+ * Is the root directory and working directory shared or is
+ * the process traced and the tracing process does not have
+ * CAP_SYS_PTRACE?
+ *
+ * In either case it is not safe to change the euid or egid
+ * unless the current process has the appropriate cap and so
+ * chaning the euid or egid was already possible.
+ */
+ need_cap = bprm->unsafe & LSM_UNSAFE_SHARE ||
+ !ptracer_capable(current, new->user_ns);
+ if (need_cap && !uid_eq(new->euid, new->uid) &&
+ (!ns_capable(new->user_ns, CAP_SETUID) ||
+ (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) {
+ new->euid = new->uid;
+ }
+ if (need_cap && !gid_eq(new->egid, new->gid) &&
+ (!ns_capable(new->user_ns, CAP_SETUID) ||
+ (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS))) {
+ new->egid = new->gid;
+ }
}
+
+ new->suid = new->fsuid = new->euid;
+ new->sgid = new->fsgid = new->egid;
}
/*
@@ -809,7 +809,7 @@ int cap_bprm_creds_from_file(struct linux_binprm *bprm)
/* Process setpcap binaries and capabilities for uid 0 */
const struct cred *old = current_cred();
struct cred *new = bprm->cred;
- bool effective = false, has_fcap = false, is_setid;
+ bool effective = false, has_fcap = false;
int ret;
kuid_t root_uid;
@@ -828,31 +828,21 @@ int cap_bprm_creds_from_file(struct linux_binprm *bprm)
if (__cap_gained(permitted, new, old))
bprm->per_clear = 1;
- /* Don't let someone trace a set[ug]id/setpcap binary with the revised
+ /* Don't let someone trace a setpcap binary with the revised
* credentials unless they have the appropriate permit.
*
* In addition, if NO_NEW_PRIVS, then ensure we get no new privs.
*/
- is_setid = __is_setuid(new, old) || __is_setgid(new, old);
-
- if ((is_setid || __cap_gained(permitted, new, old)) &&
+ if (__cap_gained(permitted, new, old) &&
((bprm->unsafe & ~LSM_UNSAFE_PTRACE) ||
!ptracer_capable(current, new->user_ns))) {
/* downgrade; they get no more than they had, and maybe less */
- if (!ns_capable(new->user_ns, CAP_SETUID) ||
- (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)) {
- new->euid = new->uid;
- new->egid = new->gid;
- }
new->cap_permitted = cap_intersect(new->cap_permitted,
old->cap_permitted);
}
- new->suid = new->fsuid = new->euid;
- new->sgid = new->fsgid = new->egid;
-
/* File caps or setid cancels ambient. */
- if (has_fcap || is_setid)
+ if (has_fcap || __is_setuid(new, old) || __is_setgid(new, old))
cap_clear(new->cap_ambient);
/*
@@ -885,10 +875,9 @@ int cap_bprm_creds_from_file(struct linux_binprm *bprm)
return -EPERM;
/* Check for privilege-elevated exec. */
- if (is_setid ||
- (!__is_real(root_uid, new) &&
- (effective ||
- __cap_grew(permitted, ambient, new))))
+ if (!__is_real(root_uid, new) &&
+ (effective ||
+ __cap_grew(permitted, ambient, new)))
bprm->secureexec = 1;
return 0;
The logic in cap_bprm_creds_from_file is difficult to follow in part because it handles both uids/gids and capabilities. That difficulty in following the code has resulted in several small bugs. Move the handling of uids/gids into bprm_fill_uid to make the code clearer. A small bug is fixed where the ambient capabilities were unnecessarily cleared when the presence of a ptracer or a shared fs_struct resulted in the setuid or setgid not being honored. This bug was not possible to leave in place with the movement of the uids and gids handling out of cap_bprm_repopultate_creds. The rest of the bugs I have tried to make more apparent but left in tact when moving the code into bprm_fill_uid. Ref: ee67ae7ef6ff ("commoncap: Move cap_elevated calculation into bprm_set_creds") Fixes: 58319057b784 ("capabilities: ambient capabilities") Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- fs/exec.c | 49 ++++++++++++++++++++++++++++++++++++-------- security/commoncap.c | 25 +++++++--------------- 2 files changed, 48 insertions(+), 26 deletions(-)