[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-api
Subject:    Re: [PATCH v6 15/16] exec: Fix dead-lock in de_thread with ptrace_attach
From:       Bernd Edlinger <bernd.edlinger () hotmail ! de>
Date:       2020-03-29 6:36:15
Message-ID: AM6PR03MB5170480FE4462D0DFF04B14BE4CA0 () AM6PR03MB5170 ! eurprd03 ! prod ! outlook ! com
[Download RAW message or body]

On 3/29/20 6:31 AM, Bernd Edlinger wrote:
> On 3/25/20 3:27 PM, Eric W. Biederman wrote:
>> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>> At a minimum the code is subtle and I don't see big fat
>> warning comments that subtle code needs to keep people
>> from using it wrong.
>>
> 
> Okay, I can add big fat warning comments, yeah.
> 

So how about that:

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 221759e..2d6b5cd 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -395,6 +395,17 @@ static int ptrace_attach(struct task_struct *task, long req
        if (mutex_lock_interruptible(&task->signal->exec_guard_mutex))
                goto out;
 
+       /*
+        * BIG FAT WARNING - Fragile code ahead.
+        * Please do not insert any code between these two
+        * if statements.  It may happen that execve has to
+        * release the exec_guard_mutex in order to prevent
+        * deadlocks.  In that case unsafe_execve_in_progress
+        * will be set.  If that happens you cannot assume that
+        * the usual guarantees implied by exec_guard_mutex
+        * are valid.  Just return -EAGAIN in that case and
+        * unlock the mutex immediately.
+        */
        retval = -EAGAIN;
        if (unlikely(task->signal->unsafe_execve_in_progress))
                goto unlock_creds;

Is that cool enough :-)


Thanks
Bernd.
[prev in list] [next in list] [prev in thread] [next in thread]