On Mon, 25 Apr 2022 16:42:12 +0200
Daniel Borkmann <daniel@xxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
