[PATCH 2/2] exec: Remove LSM_UNSAFE_SHARE

From: Kees Cook
Date: Thu Oct 06 2022 - 04:28:21 EST


With fs_struct explicitly unshared during exec, it is no longer possible
to have unexpected shared state, and LSM_UNSAFE_SHARE can be entirely
removed.

Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Eric Biederman <ebiederm@xxxxxxxxxxxx>
Cc: John Johansen <john.johansen@xxxxxxxxxxxxx>
Cc: Paul Moore <paul@xxxxxxxxxxxxxx>
Cc: James Morris <jmorris@xxxxxxxxx>
Cc: "Serge E. Hallyn" <serge@xxxxxxxxxx>
Cc: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
Cc: Eric Paris <eparis@xxxxxxxxxxxxxx>
Cc: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
Cc: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
Cc: Xin Long <lucien.xin@xxxxxxxxx>
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
Cc: Todd Kjos <tkjos@xxxxxxxxxx>
Cc: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
Cc: linux-fsdevel@xxxxxxxxxxxxxxx
Cc: linux-mm@xxxxxxxxx
Cc: apparmor@xxxxxxxxxxxxxxxx
Cc: linux-security-module@xxxxxxxxxxxxxxx
Cc: selinux@xxxxxxxxxxxxxxx
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
fs/exec.c | 17 +----------------
include/linux/security.h | 5 ++---
security/apparmor/domain.c | 5 -----
security/selinux/hooks.c | 10 ----------
4 files changed, 3 insertions(+), 34 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 7d5f63f03c58..3cd058711098 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1563,8 +1563,7 @@ EXPORT_SYMBOL(bprm_change_interp);
*/
static void check_unsafe_exec(struct linux_binprm *bprm)
{
- struct task_struct *p = current, *t;
- unsigned n_fs;
+ struct task_struct *p = current;

if (p->ptrace)
bprm->unsafe |= LSM_UNSAFE_PTRACE;
@@ -1575,20 +1574,6 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
*/
if (task_no_new_privs(current))
bprm->unsafe |= LSM_UNSAFE_NO_NEW_PRIVS;
-
- t = p;
- n_fs = 1;
- spin_lock(&p->fs->lock);
- rcu_read_lock();
- while_each_thread(p, t) {
- if (t->fs == p->fs)
- n_fs++;
- }
- rcu_read_unlock();
-
- if (p->fs->users > n_fs)
- bprm->unsafe |= LSM_UNSAFE_SHARE;
- spin_unlock(&p->fs->lock);
}

static void bprm_fill_uid(struct linux_binprm *bprm, struct file *file)
diff --git a/include/linux/security.h b/include/linux/security.h
index 1bc362cb413f..db508a8c3cc7 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -215,9 +215,8 @@ struct sched_param;
struct request_sock;

/* bprm->unsafe reasons */
-#define LSM_UNSAFE_SHARE 1
-#define LSM_UNSAFE_PTRACE 2
-#define LSM_UNSAFE_NO_NEW_PRIVS 4
+#define LSM_UNSAFE_PTRACE BIT(0)
+#define LSM_UNSAFE_NO_NEW_PRIVS BIT(1)

#ifdef CONFIG_MMU
extern int mmap_min_addr_handler(struct ctl_table *table, int write,
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 91689d34d281..1b2c0bb4d9ae 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -924,11 +924,6 @@ int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm)
goto audit;
}

- if (bprm->unsafe & LSM_UNSAFE_SHARE) {
- /* FIXME: currently don't mediate shared state */
- ;
- }
-
if (bprm->unsafe & (LSM_UNSAFE_PTRACE)) {
/* TODO: test needs to be profile of label to new */
error = may_change_ptraced_domain(new, &info);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 79573504783b..3ec80cc8ad1c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2349,16 +2349,6 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
if (rc)
return rc;

- /* Check for shared state */
- if (bprm->unsafe & LSM_UNSAFE_SHARE) {
- rc = avc_has_perm(&selinux_state,
- old_tsec->sid, new_tsec->sid,
- SECCLASS_PROCESS, PROCESS__SHARE,
- NULL);
- if (rc)
- return -EPERM;
- }
-
/* Make sure that anyone attempting to ptrace over a task that
* changes its SID has the appropriate permit */
if (bprm->unsafe & LSM_UNSAFE_PTRACE) {
--
2.34.1