2009-04-03 14:46:12

by Metzger, Markus T

[permalink] [raw]
Subject: [patch 00/20] x86, ptrace, bts, hw-branch-tracer: fixes and cleanups

--
Patches 1-5 fix races with context switching code when the branch traced task
is currently running.

In the worst case, this might cause context switch code to access freed
memory or the tracing hardware to continue tracing into a freed buffer.
Both might crash the kernel.

The first 4 patches apply to .29 using the below preparation patch.


The remaining patches fix bugs in the context of per-cpu tracing (i.e.
hw-branch-tracer) and pebs, add more selftest code, and do some cleanups.


Signed-off-by: Markus Metzger <[email protected]>
---

Index: linux-2.6.29/arch/x86/kernel/ds.c
===================================================================
--- linux-2.6.29.orig/arch/x86/kernel/ds.c 2009-04-01 17:39:55.000000000 +0200
+++ linux-2.6.29/arch/x86/kernel/ds.c 2009-04-01 17:40:42.000000000 +0200
@@ -78,8 +78,8 @@ struct bts_tracer {
struct ds_tracer ds;
/* the trace including the DS configuration */
struct bts_trace trace;
- /* buffer overflow notification function */
- bts_ovfl_callback_t ovfl;
+ /* Buffer overflow notification function: */
+ bts_ovfl_callback_t ovfl;
};

struct pebs_tracer {
Index: linux-2.6.29/arch/x86/kernel/ptrace.c
===================================================================
--- linux-2.6.29.orig/arch/x86/kernel/ptrace.c 2009-04-01 17:39:55.000000000 +0200
+++ linux-2.6.29/arch/x86/kernel/ptrace.c 2009-04-01 17:40:42.000000000 +0200
@@ -21,6 +21,7 @@
#include <linux/audit.h>
#include <linux/seccomp.h>
#include <linux/signal.h>
+#include <linux/ftrace.h>

#include <asm/uaccess.h>
#include <asm/pgtable.h>
---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2009-04-03 15:36:42

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 00/20] x86, ptrace, bts, hw-branch-tracer: fixes and cleanups


* [email protected] <[email protected]> wrote:

> Patches 1-5 fix races with context switching code when the branch
> traced task is currently running.
>
> In the worst case, this might cause context switch code to access
> freed memory or the tracing hardware to continue tracing into a
> freed buffer. Both might crash the kernel.
>
> The first 4 patches apply to .29 using the below preparation
> patch.
>
> The remaining patches fix bugs in the context of per-cpu tracing
> (i.e. hw-branch-tracer) and pebs, add more selftest code, and do
> some cleanups.

Thanks - this looks pretty acceptable. Latest -git changed a lot of
code in the same area (-mm bits went upstream), creating a lot of
conflicts.

To not prolongue this any longer (20 patches are difficult enough
already to handle) i picked up your patches and resolved the
conflicts in situ - mind having a look at the resulting
tip:tracing/hw-branch-tracing branch - does the end result look sane
to you?

It would also be nice to address Peter's feedback about the mm.h
detail and the locked-pages API - but we can do that on top.

Thanks,

Ingo

2009-04-03 17:45:25

by Markus Metzger

[permalink] [raw]
Subject: Re: [patch 00/20] x86, ptrace, bts, hw-branch-tracer: fixes and cleanups

On Fri, 2009-04-03 at 17:36 +0200, Ingo Molnar wrote:
> * [email protected] <[email protected]> wrote:
>
> > Patches 1-5 fix races with context switching code when the branch
> > traced task is currently running.
> >
> > In the worst case, this might cause context switch code to access
> > freed memory or the tracing hardware to continue tracing into a
> > freed buffer. Both might crash the kernel.
> >
> > The first 4 patches apply to .29 using the below preparation
> > patch.
> >
> > The remaining patches fix bugs in the context of per-cpu tracing
> > (i.e. hw-branch-tracer) and pebs, add more selftest code, and do
> > some cleanups.
>
> Thanks - this looks pretty acceptable. Latest -git changed a lot of
> code in the same area (-mm bits went upstream), creating a lot of
> conflicts.
>
> To not prolongue this any longer (20 patches are difficult enough
> already to handle) i picked up your patches and resolved the
> conflicts in situ - mind having a look at the resulting
> tip:tracing/hw-branch-tracing branch - does the end result look sane
> to you?

Thanks,

The branch tracing bits in tip/tracing/hw-branch-tracing look good.
The kernel boots (after changes, see below) and my tests pass.

A defconfig build fails with a compile error, though:
In file included from net/core/skbuff.c:68:
include/trace/skb.h:7: error: expected ‘)’ before ‘(’ token
.....

Changing TP~ to TP_~ (as in tip/master) seems to do the trick.


> It would also be nice to address Peter's feedback about the mm.h
> detail and the locked-pages API - but we can do that on top.

I will address them, though not immediately (vacation).


thanks and regards,
markus.

2009-04-03 17:48:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 00/20] x86, ptrace, bts, hw-branch-tracer: fixes and cleanups


* Markus Metzger <[email protected]> wrote:

> On Fri, 2009-04-03 at 17:36 +0200, Ingo Molnar wrote:
> > * [email protected] <[email protected]> wrote:
> >
> > > Patches 1-5 fix races with context switching code when the branch
> > > traced task is currently running.
> > >
> > > In the worst case, this might cause context switch code to access
> > > freed memory or the tracing hardware to continue tracing into a
> > > freed buffer. Both might crash the kernel.
> > >
> > > The first 4 patches apply to .29 using the below preparation
> > > patch.
> > >
> > > The remaining patches fix bugs in the context of per-cpu tracing
> > > (i.e. hw-branch-tracer) and pebs, add more selftest code, and do
> > > some cleanups.
> >
> > Thanks - this looks pretty acceptable. Latest -git changed a lot of
> > code in the same area (-mm bits went upstream), creating a lot of
> > conflicts.
> >
> > To not prolongue this any longer (20 patches are difficult enough
> > already to handle) i picked up your patches and resolved the
> > conflicts in situ - mind having a look at the resulting
> > tip:tracing/hw-branch-tracing branch - does the end result look sane
> > to you?
>
> Thanks,
>
> The branch tracing bits in tip/tracing/hw-branch-tracing look good.
> The kernel boots (after changes, see below) and my tests pass.
>
> A defconfig build fails with a compile error, though:
> In file included from net/core/skbuff.c:68:
> include/trace/skb.h:7: error: expected ‘)’ before ‘(’ token
> .....
>
> Changing TP~ to TP_~ (as in tip/master) seems to do the trick.

Ah. Darn - i should have merged tracing/core into it. That will ruin
bisectability... I'll redo it.

Ingo