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

Bernd Edlinger
2020-03-09T18:01:41Z

On 3/9/20 6:40 PM, Eric W. Biederman wrote:
> Bernd Edlinger <bernd.edlinger@hotmail.de> 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
>