lkml.org 
[lkml]   [2023]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [syzbot] [bpf?] [net?] BUG: unable to handle kernel NULL pointer dereference in __build_skb_around
    From
    From: Alexander Lobakin <aleksander.lobakin@intel.com>
    Date: Wed, 15 Mar 2023 13:51:23 +0100

    > From: Alexander Lobakin <aleksander.lobakin@intel.com>
    > Date: Wed, 15 Mar 2023 13:10:44 +0100
    >
    >> From: Syzbot <syzbot+e1d1b65f7c32f2a86a9f@syzkaller.appspotmail.com>
    >> Date: Wed, 15 Mar 2023 05:03:47 -0700
    >>
    >>> Hello,

    [...]

    > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
    > index 6a8b33a103a4..5b9ca36ff21d 100644
    > --- a/net/bpf/test_run.c
    > +++ b/net/bpf/test_run.c
    > @@ -217,12 +217,12 @@ static bool ctx_was_changed(struct xdp_page_head *head)
    >
    > static void reset_ctx(struct xdp_page_head *head)
    > {
    > - if (likely(!ctx_was_changed(head)))
    > - return;
    > + if (unlikely(!ctx_was_changed(head))) {

    Must be `unlikely(ctx_was_changed(head))` obviously, I've got attention
    deficit or what :D

    > + head->ctx.data = head->orig_ctx.data;
    > + head->ctx.data_meta = head->orig_ctx.data_meta;
    > + head->ctx.data_end = head->orig_ctx.data_end;
    > + }
    >
    > - head->ctx.data = head->orig_ctx.data;
    > - head->ctx.data_meta = head->orig_ctx.data_meta;
    > - head->ctx.data_end = head->orig_ctx.data_end;
    > xdp_update_frame_from_buff(&head->ctx, &head->frm);
    > }
    >
    > ---
    > Alternative version, which fixes only this particular problem, but is
    > less safe as still assumes only xdpf->data could be nulled-out. It can
    > save a bunch o'cycles on hotpath tho, thus attaching it as well:
    >
    > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
    > index 6a8b33a103a4..55789772f039 100644
    > --- a/net/bpf/test_run.c
    > +++ b/net/bpf/test_run.c
    > @@ -217,13 +217,15 @@ static bool ctx_was_changed(struct xdp_page_head *head)
    >
    > static void reset_ctx(struct xdp_page_head *head)
    > {
    > - if (likely(!ctx_was_changed(head)))
    > - return;
    > + if (unlikely(ctx_was_changed(head))) {

    (here it's correct)

    > + head->ctx.data = head->orig_ctx.data;
    > + head->ctx.data_meta = head->orig_ctx.data_meta;
    > + head->ctx.data_end = head->orig_ctx.data_end;
    > + head->frm.data = NULL;
    > + }
    >
    > - head->ctx.data = head->orig_ctx.data;
    > - head->ctx.data_meta = head->orig_ctx.data_meta;
    > - head->ctx.data_end = head->orig_ctx.data_end;
    > - xdp_update_frame_from_buff(&head->ctx, &head->frm);
    > + if (head->frm.data != head->ctx.data)
    > + xdp_update_frame_from_buff(&head->ctx, &head->frm);
    > }
    >
    > static int xdp_recv_frames(struct xdp_frame **frames, int nframes,

    Olek

    \
     
     \ /
      Last update: 2023-03-27 01:01    [W:6.503 / U:1.584 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site