netdev - Re: [syzbot] [bpf?] [net?] BUG: unable to handle kernel NULL pointer dereference in __build_skb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9acb9b6e-d998-fd7e-c370-3b6160cfda40@intel.com>
Date:   Wed, 15 Mar 2023 13:54:41 +0100
From:   Alexander Lobakin <aleksander.lobakin@...el.com>
To:     <ast@...nel.org>,
        Toke Høiland-Jørgensen <toke@...hat.com>
CC:     syzbot <syzbot+e1d1b65f7c32f2a86a9f@...kaller.appspotmail.com>,
        <bpf@...r.kernel.org>, <daniel@...earbox.net>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <hawk@...nel.org>,
        <john.fastabend@...il.com>, <kuba@...nel.org>,
        <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
        <pabeni@...hat.com>, <syzkaller-bugs@...glegroups.com>
Subject: Re: [syzbot] [bpf?] [net?] BUG: unable to handle kernel NULL pointer
 dereference in __build_skb_around

From: Alexander Lobakin <aleksander.lobakin@...el.com>
Date: Wed, 15 Mar 2023 13:51:23 +0100

> From: Alexander Lobakin <aleksander.lobakin@...el.com>
> Date: Wed, 15 Mar 2023 13:10:44 +0100
> 
>> From: Syzbot <syzbot+e1d1b65f7c32f2a86a9f@...kaller.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