linux-kernel - Re: [PATCH 5/5] ptrace: implement PTRACE

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] [day] [month] [year] [list]

Message-ID: <20110923122634.GA28898@redhat.com>
Date:	Fri, 23 Sep 2011 14:26:34 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Matt Fleming <matt@...sole-pimps.org>
Cc:	Tejun Heo <tj@...nel.org>, vda.linux@...glemail.com,
	jan.kratochvil@...hat.com, linux-kernel@...r.kernel.org,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	indan@....nu, bdonlan@...il.com, pedro@...esourcery.com
Subject: Re: [PATCH 5/5] ptrace: implement PTRACE_LISTEN

On 09/23, Matt Fleming wrote:
>
> On Tue, 2011-06-14 at 11:20 +0200, Tejun Heo wrote:
>
> [...]
>
> > +	case PTRACE_LISTEN:
> > +		/*
> > +		 * Listen for events.  Tracee must be in STOP.  It's not
> > +		 * resumed per-se but is not considered to be in TRACED by
> > +		 * wait(2) or ptrace(2).  If an async event (e.g. group
> > +		 * stop state change) happens, tracee will enter STOP trap
> > +		 * again.  Alternatively, ptracer can issue INTERRUPT to
> > +		 * finish listening and re-trap tracee into STOP.
> > +		 */
> > +		if (unlikely(!seized || !lock_task_sighand(child, &flags)))
> > +			break;
> > +
> > +		si = child->last_siginfo;
> > +		if (unlikely(!si || si->si_code >> 8 != PTRACE_EVENT_STOP))
> > +			break;
>
> I've only just noticed this. You really don't want to break out of the
> switch while holding sighand->siglock. This should read,
>
> 		if (unlikely(!si || si->si_code >> 8 != PTRACE_EVENT_STOP)) {
> 			unlock_task_sighand(child, &flags);
>			break;

OOOPS!!! Thanks... or perhaps the patch below.

This is must have for 3.1. I'll test it and send to Linus.

Good catch, thanks.

And I seem to see other "should be fixed before 3.1" problems in the
jobctl code.

Oleg.

--- x/kernel/ptrace.c
+++ x/kernel/ptrace.c
@@ -744,20 +744,17 @@ int ptrace_request(struct task_struct *c
 			break;
 
 		si = child->last_siginfo;
-		if (unlikely(!si || si->si_code >> 8 != PTRACE_EVENT_STOP))
-			break;
-
-		child->jobctl |= JOBCTL_LISTENING;
-
-		/*
-		 * If NOTIFY is set, it means event happened between start
-		 * of this trap and now.  Trigger re-trap immediately.
-		 */
-		if (child->jobctl & JOBCTL_TRAP_NOTIFY)
-			signal_wake_up(child, true);
-
+		if (likely(si && (si->si_code >> 8) == PTRACE_EVENT_STOP)) {
+			child->jobctl |= JOBCTL_LISTENING;
+			/*
+			 * If NOTIFY is set, it means event happened between start
+			 * of this trap and now.  Trigger re-trap immediately.
+			 */
+			if (child->jobctl & JOBCTL_TRAP_NOTIFY)
+				signal_wake_up(child, true);
+			ret = 0;
+		}
 		unlock_task_sighand(child, &flags);
-		ret = 0;
 		break;
 
 	case PTRACE_DETACH:	 /* detach a process that was attached. */

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