linux-kernel - Re: [RFC PATCH] ptrace: make ptrace() fail if the tracee changed its pid unexpectedly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fc796624-2660-8c2b-0956-2c9ba8281952@redhat.com>
Date:   Sat, 19 Dec 2020 16:19:31 +0000
From:   Pedro Alves <palves@...hat.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        Oleg Nesterov <oleg@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Eugene Syromiatnikov <esyr@...hat.com>,
        Jan Kratochvil <jan.kratochvil@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Simon Marchi <simon.marchi@...icios.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] ptrace: make ptrace() fail if the tracee changed its
 pid unexpectedly

On 12/17/20 11:39 PM, Eric W. Biederman wrote:

>> resume the old leader L, it resumes the post-exec thread T which was
>> actually now stopped in PTHREAD_EVENT_EXEC. In this case the
>> PTHREAD_EVENT_EXEC event is lost, and the tracer can't know that the
>> tracee changed its pid.
> 
> The change seems sensible.  I don't expect this is common but it looks
> painful to deal with if it happens.

Yeah, the debug session becomes completely messed up, as the ptracer has no
idea the process is running a new image, breakpoints were wiped out, and
the post-exec process is resumed without the ptracer having had a chance
to install new breakpoints.  I don't see any way to deal with it without
kernel help.

> 
> Acked-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> 
> I am wondering if this should be expanded to all ptrace types for
> consistency.  Or maybe we should set a flag to make this happen for
> all ptrace events.
> 
> It just seems really odd to only worry about missing this event.
> I admit this a threaded PTRACE_EVENT_EXEC is the only event we are
> likely to miss but still.

It's more about the tid stealing than the event itself.  I mean,
we lose the event because the tid changes magically without warning.
An exec is the only scenario where this happens.

> 
> Do you by any chance have any debugger/strace test cases?
> 
> I would think that would be the way to test to see if this breaks
> anything.  I think I remember strace having a good test suite.

I ran the GDB testsuite against this, no regressions showed up.

BTW, the problem was discovered by Simon Marchi when he tried to write
a GDB testcase for a multi-threaded exec scenario:

 https://sourceware.org/bugzilla/show_bug.cgi?id=26754

I've went through GDB's code looking for potential issues with the change and whether
it would affect GDBs already in the wild.  Tricky corner cases abound, but I think
we're good.  Feel free to add my ack:

Acked-by: Pedro Alves <palves@...hat.com>

Thanks,
Pedro Alves