lkml.org 
[lkml]   [2017]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH v2 1/5] ptrace: Don't wait in PTRACE_O_TRACEEXIT for exec or coredump
On 04/02, Eric W. Biederman wrote:
>
> In the case of exec and coredump which have many interesting deadlock
> opportunities

So this patch is very close to my 2/2 one-liner, except

- you removed the current->mm == current->parent->mm check

I didn't do this on purpose, because even the->core_state
is not really needed if we check ->group_exit_task, this
need more changes anyway, but I won't argue.

- With your patch we send the notification to debugger even
if we are not going to stop.

This is not wrong, but why? This is pointless, nobody rely
on SIGCHLD, if nothing else it doesn't queue.

Again, I won't argue, but this complicates both the patch
and the code for no reason. Unless I missed something.

> Keep sending the signal to the tracer so that this appears like
> the worst case where someone else sent the process a SIGKILL before
> the tracer could react. So all non-buggy tracers must support
> this case.

Well, I can't understand the changelog. Sure, debugger must support
this case, but obviously this can break things anyway.

For example. The coredumping thread must stop in PTRACE_EVENT_EXIT.
There is a tool (I don't remember its name) which does
ptrace_attach(PTRACE_SEIZE, PTRACE_O_TRACEEXIT) after the coredump
was already started, closes the pipe, and reads the registers when
this thread actually exits.

This patch or my 2/2 should not break it, ->group_exit_task will be
cleared after do_coredump(), but unfortunately something else can be
broken.

So I think the changelog should mention that yes, this is the user
visible change which _can_ break something anyway.

In short. I will be really happy if this patch comes from you, not me ;)

Oleg.

\
 
 \ /
  Last update: 2017-04-05 18:21    [W:0.110 / U:1.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site