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

[Alexander Lobakin]

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

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