Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

exec:check_unsafe_exec: kill the dead -EAGAIN and clear_in_exec logic

fs_struct->in_exec == T means that this ->fs is used by a single process
(thread group), and one of the treads does do_execve().

To avoid the mt-exec races this code has the following complications:

1. check_unsafe_exec() returns -EBUSY if ->in_exec was
already set by another thread.

2. do_execve_common() records "clear_in_exec" to ensure
that the error path can only clear ->in_exec if it was
set by current.

However, after 9b1bf12d5d51 "signals: move cred_guard_mutex from
task_struct to signal_struct" we do not need these complications:

1. We can't race with our sub-thread, this is called under
per-process ->cred_guard_mutex. And we can't race with
another CLONE_FS task, we already checked that this fs
is not shared.

We can remove the dead -EAGAIN logic.

2. "out_unmark:" in do_execve_common() is either called
under ->cred_guard_mutex, or after de_thread() which
kills other threads, so we can't race with sub-thread
which could set ->in_exec. And if ->fs is shared with
another process ->in_exec should be false anyway.

We can clear in_exec unconditionally.

This also means that check_unsafe_exec() can be void.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Oleg Nesterov and committed by
Linus Torvalds
9e00cdb0 83f62a2e

+8 -21
+8 -21
fs/exec.c
··· 1223 1223 * - the caller must hold ->cred_guard_mutex to protect against 1224 1224 * PTRACE_ATTACH 1225 1225 */ 1226 - static int check_unsafe_exec(struct linux_binprm *bprm) 1226 + static void check_unsafe_exec(struct linux_binprm *bprm) 1227 1227 { 1228 1228 struct task_struct *p = current, *t; 1229 1229 unsigned n_fs; 1230 - int res = 0; 1231 1230 1232 1231 if (p->ptrace) { 1233 1232 if (p->ptrace & PT_PTRACE_CAP) ··· 1252 1253 } 1253 1254 rcu_read_unlock(); 1254 1255 1255 - if (p->fs->users > n_fs) { 1256 + if (p->fs->users > n_fs) 1256 1257 bprm->unsafe |= LSM_UNSAFE_SHARE; 1257 - } else { 1258 - res = -EAGAIN; 1259 - if (!p->fs->in_exec) { 1260 - p->fs->in_exec = 1; 1261 - res = 1; 1262 - } 1263 - } 1258 + else 1259 + p->fs->in_exec = 1; 1264 1260 spin_unlock(&p->fs->lock); 1265 - 1266 - return res; 1267 1261 } 1268 1262 1269 - /* 1270 - * Fill the binprm structure from the inode. 1263 + /* 1264 + * Fill the binprm structure from the inode. 1271 1265 * Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes 1272 1266 * 1273 1267 * This may be called multiple times for binary chains (scripts for example). ··· 1445 1453 struct linux_binprm *bprm; 1446 1454 struct file *file; 1447 1455 struct files_struct *displaced; 1448 - bool clear_in_exec; 1449 1456 int retval; 1450 1457 1451 1458 /* ··· 1476 1485 if (retval) 1477 1486 goto out_free; 1478 1487 1479 - retval = check_unsafe_exec(bprm); 1480 - if (retval < 0) 1481 - goto out_free; 1482 - clear_in_exec = retval; 1488 + check_unsafe_exec(bprm); 1483 1489 current->in_execve = 1; 1484 1490 1485 1491 file = open_exec(filename); ··· 1546 1558 } 1547 1559 1548 1560 out_unmark: 1549 - if (clear_in_exec) 1550 - current->fs->in_exec = 0; 1561 + current->fs->in_exec = 0; 1551 1562 current->in_execve = 0; 1552 1563 1553 1564 out_free: