linux-kernel - Re: [PATCH 10/10] ptrace: implement group stop notification for ptracer

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, 19 May 2011 18:57:22 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	jan.kratochvil@...hat.com, vda.linux@...glemail.com,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org, indan@....nu, bdonlan@...il.com
Subject: Re: [PATCH 10/10] ptrace: implement group stop notification for
 ptracer

Hey,

On Thu, May 19, 2011 at 06:32:46PM +0200, Oleg Nesterov wrote:
> > +static void ptrace_trap_notify(struct task_struct *t)
> > +{
> > +	siginfo_t *si = t->last_siginfo;
> > +
> > +	WARN_ON_ONCE(!(t->ptrace & PT_SEIZED));
> > +	assert_spin_locked(&t->sighand->siglock);
> > +
> > +	/*
> > +	 * @t is being ptraced and new SEIZE behavior is in effect.
> > +	 * Schedule sticky trap which will clear on the next GETSIGINFO.
> > +	 */
> > +	t->jobctl |= JOBCTL_TRAP_NOTIFY;
> 
> This is also set by do_signal_stop(). Cleared by PTRACE_GETSIGINFO.
> 
> How can this work? Doesn't this mean PTRACE_GETSIGINFO becomes mandatory
> before PTRACE_CONT? IOW, unless the tracee does PTRACE_GETSIGINFO to clear
> this bit, PTRACE_CONT just leads to another trap, no?

Yes, group stop state change raises a sticky trap condition which is
cleared by GETSIGINFO.

> > +	if (task_is_traced(t) && si && si->si_code == PTRACE_STOP_SI_CODE) {
> 
> OK, this PTRACE_STOP_SI_CODE check is clear. But the same check in
> ptrace_check_attach() looks confusing, why can't we set BLOCK_NOTIFY
> unconditionally?

It's an optimization.  If we set the flag, we'll have to acquire
siglock and clear it before returning to userland.  Setting
BLOCK_NOTIFY isn't necessary unless tracee is in TRAP_STOP, so...

> > +		t->jobctl |= JOBCTL_TRAPPING;
> > +		if (!(t->jobctl & JOBCTL_BLOCK_NOTIFY))
> > +			signal_wake_up(t, true);
> 
> Could you please remind me why we can't avoid the awful ptrace_wait_trapping()
> in do_wait() paths? Assuming that ptrace_check_attach() does this. I got lost
> a bit.

Please consider the following scenario.

1. Tracee is in group stop and stops at TRAP_STOP notifying the
   tracer.

2. Tracer does WNOWAIT wait(2) and determines that the tracee is
   trapped in TRAP_STOP.

3. Something generates SIGCONT which finishes the group stop and
   triggers the notification re-trapping.

4. While tracee is re-trapping, tracer issues WNOHANG wait(2)
   expecting it to succeed but the tracee could be RUNNING for re-trap
   thus failing WNOHANG wait(2).

> So. The tracee reports PTRACE_EVENT_STOP, debugger issues a lot of PTRACE_
> requests. The tracee can report another trap "in between". Looks confusing...
> Perhaps I need to get used to it.

Yes, and the reported trap doesn't interfere with wait(2) or ptrace
requests.  Trapping is the only event notification mechanism available
from tracee to tracer and we already have interface to wait and poll
them, so I think it makes sense to use it.

Thank you.

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