linux-kernel - Re: [PATCH 1/1] ptrace: fix ptrace_signal() && STOP

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]

Date:	Thu, 14 Jul 2011 08:45:06 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	vda.linux@...glemail.com, jan.kratochvil@...hat.com,
	pedro@...esourcery.com, indan@....nu, bdonlan@...il.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] ptrace: fix ptrace_signal() && STOP_DEQUEUED
 interaction

Hello, Oleg.

On Wed, Jul 13, 2011 at 08:33:22PM +0200, Oleg Nesterov wrote:
> > 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.

Yeah, we can't change it.  I was just thinking out loud.  Sending
signals from ptrace code path seems generally wrong.  In this case, if
the signal becomes blocked inbetween, the signal gets sent again.
Seeing the same signal being delivered twice can be confusing.
Moreover, SIGCONT can be blocked and the action of 'sending' it has
side effects, so re-sending it is simply wrong.  This should have been
ack/nack for the action to take.

Anyways, not much point in ranting about it at this point, I guess.

> > 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?

Never mind.  I for some reason thought flipping the flag would make
the extra step in ptrace_signal() unnecessary.  We need to clear it
all the same so it doesn't really improve anything.  I think the
current version should be fine (maybe the comment can be beefed up a
bit?).

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/