linux-kernel - Re: [PATCH 1/5] ptrace: Prepare to fix racy accesses on task breakpoints

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: <20110504182158.GC1804@nowhere>
Date:	Wed, 4 May 2011 20:22:01 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Will Deacon <will.deacon@....com>,
	Prasad <prasad@...ux.vnet.ibm.com>,
	Paul Mundt <lethal@...ux-sh.org>,
	"v2.6.33.." <stable@...nel.org>, Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH 1/5] ptrace: Prepare to fix racy accesses on task
 breakpoints

On Wed, May 04, 2011 at 08:31:06AM +0200, Ingo Molnar wrote:
> 
> (Linus and Andrew Cc:-ed as well)
> 
> * Frederic Weisbecker <fweisbec@...il.com> wrote:
> 
> > When a task is traced and is in a stopped state, the tracer
> > may execute a ptrace request to examine the tracee state and
> > get its task struct. Right after, the tracee can be killed
> > and thus its breakpoints released.
> > This can happen concurrently when the tracer is in the middle
> > of reading or modifying these breakpoints, leading to dereferencing
> > a freed pointer.
> > 
> > Hence, to prepare the fix, create a generic breakpoint reference
> > holding API. When a reference on the breakpoints of a task is
> > held, the breakpoints won't be released until the last reference
> > is dropped. After that, no more ptrace request on the task's
> > breakpoints can be serviced for the tracer.
> > 
> > Reported-by: Oleg Nesterov <oleg@...hat.com>
> > Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> 
> Ok, this series looks a bit scary - and this ptrace.h change does not have 
> Oleg's Acked-by. (the arch bits all have maintaner Acked-by's)
> 
> The changes look a bit ugly as well: beyond the ugly ifdeffery, we have 
> ptrace.h::ptrace_init_task(), which is only used in 
> tracehook.h::tracehook_finish_clone() which is only used in 
> kernel/fork.c::copy_process().
> 
> That's two levels of obfuscation to do something rather simple - i think we 
> should get rid of the tracehook.h redirections, it did not work out in the end 
> as a method of capturing events - ftrace TRACE_EVENT() seems better structured 
> and more maintainable.
> 
> But i guess we could live with this fix for v2.6.39, if neither Oleg nor Linus 
> and Andrew are hating this further complication of the ptrace mess enough to 
> NAK it. Thoughts?
> 
> Plus, i'd really love it if you did some stress-testing of this change of a 
> mixed ptrace breakpoints and perf breakpoints workload, on some sufficiently 
> SMP box. gdb's hbreak is a very low freq way of testing thus such regressions 
> take ages to be reported.

But the perf breakpoints (those created using perf syscall) are not touched
at all by this patch. Only the ptrace ones.

What I can stress test is trying some ptrace breakpoint request and at the
same time SIGKILL the child, which is the only way to reproduce the bug
supposed to be fixed. And run that in a loop for one night. I'll try that.
--
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/