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

[Bernd Edlinger]

On 3/9/20 6:40 PM, Eric W. Biederman wrote:
> Bernd Edlinger <bernd.edlinger@xxxxxxxxxx> writes:
> 
>> On 3/8/20 10:38 PM, Eric W. Biederman wrote:
>>>
>>> The cred_guard_mutex is problematic.  The cred_guard_mutex is held
>>> over the userspace accesses as the arguments from userspace are read.
>>> The cred_guard_mutex is held of PTRACE_EVENT_EXIT as the the other
>                                 ^ over
>>
>> ... is held while waiting for the trace parent to handle PTRACE_EVENT_EXIT
>> or something?
> 
> Yes.  Let me see if I can phrase that better.
> 
>> I wonder if we also should mention that
>> it is held while waiting for the trace parent to
>> receive the exit code with "wait"?
> 
> I don't think we have to spell out the details of how it all works,
> unless that makes things clearer.  Kernel developers can be expected
> to figure out how the kernel works.  The critical thing is that it is
> an indefinite wait for userspace to take action.
> 
> But I will look.
> 
>>> threads are killed.  The cred_guard_mutex is held over
>>> "put_user(0, tsk->clear_child_tid)" in exit_mm().
>>>
>>> Any of those can result in deadlock, as the cred_guard_mutex is held
>>> over a possible indefinite userspace waits for userspace.
>>>
>>> Add exec_update_mutex that is only held over exec updating process
>>
>> Add ?
> 
> Yes.  That is what the change does: add exec_update_mutex.
> 

I just kind of missed the "subject" in this sentence,
like "This patch adds an exec_update_mutex that is ..."
but english is a foreign language for me, so may be okay as is.


Bernd.

>>> with the new contents of exec, so that code that needs not to be
>>> confused by exec changing the mm and the cred in ways that can not
>>> happen during ordinary execution of a process.
>>>
>>> The plan is to switch the users of cred_guard_mutex to
>>> exec_udpate_mutex one by one.  This lets us move forward while still
>>
>> s/udpate/update/
> 
> Yes.  Very much so.
> 
> Eric
>