Re: kprobe fault handling

[Suparna Bhattacharya <suparna at in dot ibm dot com>]

Recalling the way this was supposed to work ... AFAICR

There are 2 main situations where one could land up into
the page fault handler during kprobe processing:

(1) We have completed the probe handler and are single-stepping the probed
    instruction. The original instruction being single-stepped causes a
    page fault. This would typically happen if that instruction issues a
    memory access that is paged out.

(2) We are executing the probe handler, and an instruction in the probe
    handler causes a page fault. This could, for example, happen if the
    probe handler attempts to access a user-space address by issuing
    copy_from_user or get_user etc, and that address is currently
    paged out or invalid.

The check KPROBE_HIT_SS enables us to distinguish between these two cases.

If set, it indicates that (1) must have occured. In which case, we want
normal page fault handling to continue to that the data accessed by the
instruction is paged in, and instruction execution continue after
fault handling as would happen in normal execution. However, since
this can cause a sleep, other probe hits etc, we turn off kprobes
related state (hence the resume execution) before letting it continue. Once
the data is paged in, the page fault handler would resume execution from
the original probed address and thus trap again on the breakpoint instruction.
So we encounter the probe handler a second time, and this time single-stepping
would succeed since the address accessed by the instruction is already
in memory, and finally we would get a single-step trap and the post handler
would be executed. 

In KPROBE_HIT_SS is not set, we enter case (2). Now since we are (or at
least were) running without interrupts, the default behaviour was to
soft-fail the access, i.e. cause the copy_from_user/get_user call in
the probe handler to fail with -EFAULT. How does this happen ? The
in_atomic() check or its equivalent in do_page_fault succeeds and hence
control jumps to bad_area_nosemaphore where it finds that the access
happened in kernel mode and hence moves to no_context, where
fixup_exception() takes care of the rest.
More sophisticated or customised behaviour could be achieved by providing
a ->fault_handler(), we could play some neat tricks (like Richard mentioned)
perform fixups etc.

I hope that makes it a little clearer.

Could you now help me understand what is broken now, if at all ?

Regards
Suparna

On Tue, Feb 07, 2006 at 11:49:22AM -0800, Jim Keniston wrote:
> On Tue, 2006-02-07 at 09:51, Martin Hunt wrote:
> > On Tue, 2006-02-07 at 09:31 -0800, Jim Keniston wrote:
> > [...]
> > > > > And that's it. kprobe_fault_handler returns 0.  No call to
> > > > > fixup_exceptions()!  So do_page_fault() will have to do the fixups, but
> > > > > first it will print nasty might_sleep warnings and maybe actually sleep!
> > > 
> > > You have the right idea of running fixup_exceptions() in the fault
> > > handler, to prevent do_page_fault() from attempting its demand-paging
> > > stuff.  I just think it's OK to have the user's fault handler, rather
> > > than kprobe_fault_handler, run fixup_exceptions.
> > 
> > I think if the user's fault handler does not handle the page fault,
> > kprobes should attempt to do so. Returning without handling the page
> > fault could leave the system in a bad state. So your proposal would mean
> > it is necessary to specify a fault handler for any kprobe that might
> > attempt user-space access or otherwise trigger a page fault, otherwise
> > the system could crash.
> 
> I just had a long chat with Richard Moore about this whole topic.  I
> agree with you on this, and I think Richard would, too.
> 
> So unless there's a user-specified handler and that handler specifies
> (by returning 1) that it has handled the exception,
> kprobe_fault_handler() should run fixup_exception(), right?
> 
> Now I'm looking, later in that function, at the code (on i386) where we
> handle an exception while single-stepping.  I don't think
> resume_execution() is the right thing to do here.  We haven't
> successfully executed the probed instruction, and the eip still points
> at that instruction, right?  I think we're just hosed at this point. 
> Comments?
> 
> > 
> > Any more importantly, fixup_exception() and all the exception table code
> > are currently not exported so cannot be called from a module.
> 
> It's hard to argue with that. :-)
> 
> > 
> > Martin
> 
> Jim
> 
>