Re: [PATCH bpf] x86/kprobes: Fix KRETPROBES when CONFIG_KRETPROBE_ON_RETHOOK is set

[Masami Hiramatsu <mhiramat@kernel.org>]

On Mon, 25 Apr 2022 16:42:12 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> On 4/22/22 6:40 PM, Adam Zabrocki wrote:
> > [PATCH bpf] x86/kprobes: Fix KRETPROBES when CONFIG_KRETPROBE_ON_RETHOOK is set
> > 
> > The recent kernel change "kprobes: Use rethook for kretprobe if possible",
> > introduced a potential NULL pointer dereference bug in the KRETPROBE
> > mechanism. The official Kprobes documentation defines that "Any or all
> > handlers can be NULL". Unfortunately, there is a missing return handler
> > verification to fulfill these requirements and can result in a NULL pointer
> > dereference bug.
> > 
> > This patch adds such verification in kretprobe_rethook_handler() function.
> > 
> > Fixes: 73f9b911faa7 ("kprobes: Use rethook for kretprobe if possible")
> > Signed-off-by: Adam Zabrocki <pi3@pi3.com.pl>
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> I don't mind if this fix gets routed via bpf tree if all parties are okay with
> it (Masami? Steven?). Just noting that there is currently no specific dependency
> in bpf tree for it, but if it's easier to route this way, happy to take it.

Yeah, I and Steve talked about it and he suggested this to be merged
via BPF tree since the original commit came from the BPF tree.

Thank you,

> 
> > ---
> >   kernel/kprobes.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index dbe57df2e199..dd58c0be9ce2 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -2126,7 +2126,7 @@ static void kretprobe_rethook_handler(struct rethook_node *rh, void *data,
> >   	struct kprobe_ctlblk *kcb;
> >   
> >   	/* The data must NOT be null. This means rethook data structure is broken. */
> > -	if (WARN_ON_ONCE(!data))
> > +	if (WARN_ON_ONCE(!data) || !rp->handler)
> >   		return;
> >   
> >   	__this_cpu_write(current_kprobe, &rp->kp);
> > 
> 
> Thanks,
> Daniel


-- 
Masami Hiramatsu <mhiramat@kernel.org>