Hi Robin, On Thu, 17 Oct 2019 16:31:42 +0100 Robin Murphy <[email protected]> wrote:
> The short-circuit call to fixup_exception() from kprobe_fault_handler() > poses a problem now that the former wants to consume the fault address > too, since the common kprobes API offers us no way to pass it through. > Fortunately, however, it works out to be unnecessary: Thank you for pointing it out! > > - uaccess instructions themselves are not probeable, so at most we > should only ever expect to take a fixable fault from the pre or post > handlers. Right. If it is not fixable, we should handle it as a kernel bug. (to avoid it we can use probe_kernel_read() in kprobe handler) > - the pre and post handler run with preemption disabled, thus for any > fault they may cause, an unhandled return from kprobe_page_fault() > will proceed directly to __do_kernel_fault() thanks to the > faulthandler_disabled() check. OK, this is reasonable. > - __do_kernel_fault() will immediately call fixup_exception() unless > we're in an EL1 instruction abort, and if we've somehow taken one of > those on what we think is the middle of a uaccess routine, then the > world is already very on fire. OK, this looks good to me. Reviewed-by: Masami Hiramatsu <[email protected]> Thank you! > > Thus we can reasonably drop the call from kprobe_fault_handler() and > leave uaccess fixups to the regular flow. > > Signed-off-by: Robin Murphy <[email protected]> > --- > arch/arm64/kernel/probes/kprobes.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/arch/arm64/kernel/probes/kprobes.c > b/arch/arm64/kernel/probes/kprobes.c > index c4452827419b..422fbd5c6c55 100644 > --- a/arch/arm64/kernel/probes/kprobes.c > +++ b/arch/arm64/kernel/probes/kprobes.c > @@ -334,13 +334,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, > unsigned int fsr) > */ > if (cur->fault_handler && cur->fault_handler(cur, regs, fsr)) > return 1; > - > - /* > - * In case the user-specified fault handler returned > - * zero, try to fix up. > - */ > - if (fixup_exception(regs)) > - return 1; > } > return 0; > } > -- > 2.21.0.dirty > -- Masami Hiramatsu <[email protected]>
