Re: [PATCHv4] exec: Fix a deadlock in ptrace

[Christian Brauner <christian.brauner@xxxxxxxxxx>]

On Tue, Mar 03, 2020 at 11:23:31AM +0000, Bernd Edlinger wrote:
> On 3/3/20 11:34 AM, Bernd Edlinger wrote:
> > On 3/3/20 9:58 AM, Christian Brauner wrote:
> >> So one issue I see with having to reacquire the cred_guard_mutex might
> >> be that this would allow tasks holding the cred_guard_mutex to block a
> >> killed exec'ing task from exiting, right?
> >>
> > 
> > Yes maybe, but I think it will not be worse than it is now.
> > Since the second time the mutex is acquired it is done with
> > mutex_lock_killable, so at least kill -9 should get it terminated.
> > 
> 
> 
> 
> >  static void free_bprm(struct linux_binprm *bprm)
> >  {
> >  	free_arg_pages(bprm);
> >  	if (bprm->cred) {
> > +		if (!bprm->called_flush_old_exec)
> > +			mutex_lock(&current->signal->cred_guard_mutex);
> > +		current->signal->cred_locked_for_ptrace = false;
> >  		mutex_unlock(&current->signal->cred_guard_mutex);
> 
> 
> Hmm, cough...
> actually when the mutex_lock_killable fails, due to kill -9, in flush_old_exec
> free_bprm locks the same mutex, this time unkillable, but I should better do
> mutex_lock_killable here, and if that fails, I can leave cred_locked_for_ptrace,
> it shouldn't matter, since this is a fatal signal anyway, right?

I think so, yes.