lkml.org 
[lkml]   [2011]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/1] ptrace: fix ptrace_signal() && STOP_DEQUEUED interaction
Hi Tejun,

On 07/13, Tejun Heo wrote:
>
> Sorry about the long delay.

No, no, you shouldn't use this mantra. I hold the copyright!

> On Thu, Jul 07, 2011 at 09:03:24PM +0200, Oleg Nesterov wrote:
> > Without the patch it hangs. After the patch SIGSTOP "injected" by the
> > tracer is not ignored and stops the tracee.
>
> I always felt the ability to 'inject' different signal there is rather
> useless and prone to induce weird issues. It would be better if
> ptrace_signal() is part of signal delivery action after all the checks
> so that the ptracer says whether to proceed with the action or not but
> no more. Well...

Oh, probably. If the tracer wants a different signr, it can simply do
tkill() + PTRACE_CONT(0). I agree. Although perhaps this is needed for
gdb, I dunno. But we can't change this.

And, I'd like to clarify... It is not that I think it is that important
to ensure PTRACE_CONT(SIGSTOP) will actually stop the tracee if the tracer
changes the original signal. I simply do not know if it is used this way.

The only important thing, imho, the behaviour shouldn't depend on
/dev/random, with this patch it is at least clearly defined.

> > So lets add STOP_DEQUEUED _before_ we report the signal. It has no effect
> > unless sig_kernel_stop() == T after the tracer resumes us, and in the
> > latter case the pending STOP_DEQUEUED means no SIGCONT in between, we
> > should stop.
>
> Anyways, yes, this seems to be a nice improvement but it looks very
> weird (and difficult to comprehend) to be setting STOP_DEQUEUED
> unconditionally in ptrace_signal().

Yeeees, agreed. I even added the comment to explain this weirdness.

> Wouldn't it be better to flip the
> flag so that we have CONT_RECEIVED before doing this?

May be. You know, I thought about this when I did ee77f075
"signal: Turn SIGNAL_STOP_DEQUEUED into GROUP_STOP_DEQUEUED".

Or may be we can simply rename it into STOP_ALLOWED. In this case
we can even set it unconditionally before dequeue_signal().


Anyway, whatever we do, this patch doesn't complicate the
CONT_RECEIVED/STOP_ALLOWED change. Can't we do this later?

Oleg.



\
 
 \ /
  Last update: 2011-07-13 20:39    [W:0.051 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site