Re: [GIT PULL] Please pull proc and exec work for 5.7-rc1

Linus Torvalds
2020-04-28T20:36:05Z

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

On Tue, Apr 28, 2020 at 12:08 PM Oleg Nesterov <oleg@redhat.com> wrote:
>
> Oops. I can update that old patch but somehow I thought there is a better
> plan which I don't yet understand...

I don't think any plan survived reality.

Unless we want to do something *really* hacky.. The attached patch is
not meant to be serious.

> And, IIRC, Jan had some ideas how to rework the new creds calculation in
> execve paths to avoid the cred_guard_mutex deadlock?

I'm not sure how you'd do that.

Execve() fundamentally needs to serialize with PTRACE_ATTACH somehow,
since the whole point is that "tsk->ptrace" changes how the
credentials are interpreted.

So PTRACE_ATTACH doesn't really _change_ the credentials, but it very
much changes what execve() will do with them.

But I guess we could do a "if somebody attached to us while we did the
execve(), just repeat the whole thing"

Jann, what was your clever idea? Maybe it got lost in the long thread..

Linus

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 1319 bytes --]

kernel/ptrace.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 43d6179508d6..ebbc9876914b 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -31,6 +31,7 @@
#include <linux/cn_proc.h>
#include <linux/compat.h>
#include <linux/sched/signal.h>
+#include <linux/delay.h>

#include <asm/syscall.h> /* for syscall_get_* */

@@ -390,10 +391,24 @@ static int ptrace_attach(struct task_struct *task, long request,
* Protect exec's credential calculations against our interference;
* SUID, SGID and LSM creds get determined differently
* under ptrace.
+ *
+ * Don't wait forever on the credential lock if the target is
+ * going through an execve.
+ *
+ * Whatever. We don't have "mutex_lock_interruptible_timeout()".
+ * But this would be a disgusting hack even with it.
*/
- retval = -ERESTARTNOINTR;
- if (mutex_lock_interruptible(&task->signal->cred_guard_mutex))
- goto out;
+ for (;;) {
+ if (mutex_trylock(&task->signal->cred_guard_mutex))
+ break;
+ retval = -ERESTARTNOINTR;
+ if (signal_pending(current))
+ goto out;
+ retval = -EAGAIN;
+ if (task->in_execve)
+ goto out;
+ msleep_interruptible(100);
+ }

task_lock(task);
retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);