On Fri, Nov 20, 2020 at 05:14:18PM -0600, Eric W. Biederman wrote: > @@ -1805,8 +1808,12 @@ static int bprm_execve(struct linux_binprm *bprm, > bprm->file = file; > /* > * Record that a name derived from an O_CLOEXEC fd will be > - * inaccessible after exec. Relies on having exclusive access to > - * current->files (due to unshare_files above). > + * inaccessible after exec. This allows the code in exec to > + * choose to fail when the executable is not mmaped into the > + * interpreter and an open file descriptor is not passed to > + * the interpreter. This makes for a better user experience > + * than having the interpreter start and then immediately fail > + * when it finds the executable is inaccessible. > */ > if (bprm->fdpath && > close_on_exec(fd, rcu_dereference_raw(current->files->fdt))) We do not have rcu_read_lock() here. What would happen if * we get fdt * another thread does e.g. dup2() with target descriptor greater than the current size * old fdt gets copied and (RCU-delayed) freed * nobody is holding rcu_read_lock(), so it gets really freed * we read a bitmap from the already freed sucker It's a narrow window, but on SMP KVM it's not impossible to hit; if you have preemption enabled, the race window is not so narrow even when running on bare metal. In the mainline it is safe, but only because the damn thing is guaranteed to be _not_ shared with any other thread (which is what the comment had been about). Why not simply say if (bprm->fdpath && get_close_on_exec(fd)) anyway?
