Re: [PATCH v2 5/5] exec: Add a exec_update_mutex to replace cred_guard_mutex

Kees Cook [off-list ref] writes:

On Mon, Mar 09, 2020 at 02:02:37PM -0500, Eric W. Biederman wrote:

quoted

    exec: Add exec_update_mutex to replace cred_guard_mutex
    
    The 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.

quoted

[...]
    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 <redacted>

Eric