(WARNING: I've browsed the ARMv7 breakpoints implementation
but I may have an erratic/incomplete understanding, then parts
of this review might make little sense)




What do you mean here by unpredictable? It would be a pity to limit the



Doh! That must explain the problem with DFAR...





Ok, alternatively, why not having the control register in the arch
breakpoint structure:

u32   __reserved : 19,
        len             :  7,
        type           :  2,
        privilege      :  2,
        enabled      :  1;

It will avoid you to play with bitwise operations that may scale into dirty
and error prone as you may support more features from the control register
(link, mask, etc...).

So in the future if you want to support more things, you can just split out
the __reserved field into the features it has, depending on the ARM versions.

Ah and it will make ptrace support easier: the user writes into the val/ctrl
fields directly as if they were the true registers, then you can just validate


You seem to make a wrong assumption here.
This is not because the breakpoint is task-bound that we don't
want it to trigger on the kernel. We may want to trigger breakpoints
on tasklist_lock accesses from a given task.

The only case for which we don't want it to trigger on the kernel
is for ptrace breakpoints.

I guess I should remove this tsk parameter as it makes the things
only confusing. We should simply create ptrace breakpoints with
bp->attr.exclude_kernel set to 1.

Because when bp->attr.exclude_kernel is set to 1, then it truly makes sense
to think about user and supervisor privileges, just to avoid to filter the
event in the software level. Note we do this filtering on software level,
but it going to be less costly if done from hardware.

Even if you don't support ptrace right now, we can define a kernel exluded
breakpoint from perf syscall.





We don't need this callback anymore, it has been removed because
of ptrace corner ...
[ message continues ]

Hi Frederic,

Many thanks for taking a look at the these patches, I appreciate the
feedback.

It looks like you've understood it correctly. Actually, I have
a disclaimer of my own; I'm using a new mail client in the hope
By unpredictable I mean that the value in the DFAR is not defined to
correspond to the causative address in any way. This isn't the case
for a standard data abort, but it is in the case of a watchpoint
I know, it makes life a lot harder for us. The most annoying thing is that
I'm yet to find a real implementation that *doesn't* set the DFAR to what
I'm unsure about this. As mainline stands, ARM has no ptrace support for the
hardware breakpoint registers. This means that we could expose the
hardware resources in an idealised fashion via ptrace so that if the
interface varies between CPUs, userspace doesn't need to care. I had a
crack at this with another patch in the series here:

Surely we can't have breakpoints triggering on *any* part of the
breakpoint handling code path? Otherwise we'll just get stuck. That's

Sounds good to me. That also means that it's one less thing for the arch
Ok, I'll remove it.

I'll take a look at the new constraints and allocation patches you
posted this morning and then adjust these patches accordingly.

Thanks,

Will

--

[ message continues ]

You mean that sometimes DFAR doesn't have the true ip origin of the
exception?
May be you can check the trapped regs to find the instruction pointer



Ah, indeed if you need to abstract out various ARM versions, that's quite sensitive.



Ah indeed you really need to protect against recursion.

But I'm unclear about the difference between Supervisor and System.
May be System means the common kernel ring? And you enter into
Supervisor when an exception triggers?

Do these exceptions also concern page faults and not only debug




Thanks.

--

[ message continues ]

Hello Frederic,


There are two related issues here:

1.) A watchpoint can be synchronous or asynchronous, you can determine which when
    you take the exception. If the watchpoint is synchronous, then yes, we can look
    at the trapped regs to find the causative instruction. Then we would need to
    disassemble the instruction and emulate the address generation part [which may
    also need the trapped regs] in order to calculate the faulting data address.
    If the watchpoint exception is asynchronous, we can read the WFAR register to
    find the address of the causative instruction. Unfortunately, we could still
    be screwed in this case because we won't be able to emulate the address
    calculation for a register-relative load/store. Thankfully, v7 cores only
    generate synchronous watchpoints [apart from some very early revisions which
    you won't see in the wild] but v6 cores are the opposite; they only generate
    asynchronous watchpoint exceptions.

2.) For a standard data abort, the DFAR is updated to contain the data address which
    caused the fault. Ideally this would also be the case for watchpoint exceptions
    because then we wouldn't have to disassemble anything [and as I mentioned
    previously, I'm yet to find a real implementation that doesn't set the DFAR on

It's not so much about necessity, but I think it would be nice if userspace
didn't have to care so much about the breakpoint/watchpoint debug hardware on the
target. Since we don't have any backwards compatibility issues [because ptrace in
mainline has never supported hardware breakpoints on ARM] we can define a stable

I'm unsure about how to provide full protection though. You could still
set a breakpoint on the breakpoint handling code that exists outside of the
exception text. Do other archs suffer from this problem? I'm tempted

Sorry, I think my ARM_BREAKPOINT_SUPER #define is confusing. I'll rename it
to ARM_BREAKPOINT_PRIV because the hardware only distinguishes between

I ...
[ message continues ]

Yeah, looking at how this is implemented across versions, it seems
that they all share the same registers, but early versions have
unfeatured reserved bytes in control registers that are filled
with new features over time in subsequent versions.

It looks like the things are then forward compatible but not
backward.

Isn't that enough to provide a ptrace interface? I mean you can't do
much magic here. Having something that works everywhere would consist


In x86 we just write the control register to disable the breakpoints
in the exception handler. (I suspect we don't do it early enough though).

You can do the same in ARM.

Currently, there is no true security flow because if a breakpoint
is task bound, x86 refuse breakpoints in kernel addresses.
But as I noticed before, this is a wrong behaviour as we also
want to be able to trace kernel var accesses while in a given task
context, except for ptrace.

I should just add a CAP_SYS_ADMIN() && !ptrace check for kernel




If I understand you correctly,

sync = breakpoint triggers before the instruction is executed,
       trap return to the faulting instruction
async = breakpoint triggers after the instruction is executed

Then yeah, the current breakpoint framework only supports async
ones.

If an arch has no choice but using sync breakpoints, it needs
to handle that. It seems PowerPc has a similar problem, may be
have a look at Prasad's patches.

If there is something that can be handled from the generic
layer to help solving this problem, I'll try something.

--

[ message continues ]

Hi Frederic,



I suppose the ptrace interface will involve bit-packing at some point anyway,
so mirroring the hardware configuration might not be as bad as I initially thought.
The hardware has some slightly more esoteric features [breakpoint linking, context




Could you give me a pointer to these patches please [I can't see them on LKML]?
I *think* PowerPC is lucky enough to have hardware single step, so handling
synchronous breakpoints isn't *too* difficult. In ARM we have to work out the
address of the following instruction, which is quite tricky because it could be a
branch instruction [from a selection of instruction sets] and also be predicated.
GDB can handle this, but the Kernel doesn't have the capability to do so [there's

Unfortunately this is very much an arch problem. As a half-way house, I might
implement only the ptrace part of hw-breakpoint for ARM. That way we can
force the client to handle stepping over the breakpoint and we also won't have
to worry about recursive breakpoints in the Kernel.

Cheers,

Will



--

[ message continues ]

In fact, I guess watchpoint to breakpoint linking is something that userspace
can use. But the context ID linking....well I haven't understood this part



Speaking about this made me wonder about the current state in x86.
For now it's safe because task bound breakpoint only allow userspace
addresses.
But this is not right because we want task bound to be able to trigger
on kernel. Once we'll allow this, we need a bit of policy.

Because yeah we can protect against recursion, but always partially.
The only solution to really protect would be disabling the breakpoint
as the very first operation in the trap handler.

But that's ugly and may be costly as well.

So what I'm going to do is to simply check attr.exclude_kernel on
breakpoint creation and look if the address is in the kernel space.

So that if you want to crash your machine why lurking at the very
tight window of trap handler possible recursion, you need to be
root :)

Because when a perf event registers and doesn't have exclude_kernel,
you need to be admin.

Fortunately we don't have this security hole with the current rules,
but I'll try to make this check more generic (will require archs that


I don't remember exactly how this is handled. I remember a story about
a one-shot breakpoint, ie: that triggers only once.

Anyway, I have yet to review the latest version:

http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-April/081714.html
http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-April/081715.html

Yeah, we'll see what we can do.

--

[ message continues ]