Kees Cook keescook@chromium.org writes:
On Mon, Mar 09, 2020 at 02:02:37PM -0500, Eric W. Biederman wrote:
exec: Add exec_update_mutex to replace cred_guard_mutexThe cred_guard_mutex is problematic as it is held over possibly indefinite waits for userspace. The possilbe indefinite waits for userspace that I have identified are: The cred_guard_mutex is held in PTRACE_EVENT_EXIT waiting for the tracer. The cred_guard_mutex is held over "put_user(0, tsk->clear_child_tid)" in exit_mm(). The cred_guard_mutex is held over "get_user(futex_offset, ...") in exit_robust_list. The cred_guard_mutex held over copy_strings.
I suspect you're not trying to make a comprehensive list here, but do you want to mention seccomp too (since it's yet another weird case).
I was calling out all of the places I have found so far where cred_guard_mutex is held over waiting for userspace to maybe do something. Those places are what cause our deadlocks.
[...] Holding a mutex over any of those possibly indefinite waits for userspace does not appear necessary. Add exec_update_mutex that will just cover updating the process during exec where the permissions and the objects pointed to by the task struct may be out of sync.
Should the specific resources be pointed out here? creds, mm, ... ?
But otherwise, yup, looks sane:
Probably not. The design is if exec changes it we will hold the cred_guard_mutex over it, so things are semi-atomic.
Reviewed-by: Kees Cook keescook@chromium.org
Eric