netdev - Re: [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 10 Mar 2022 16:05:11 -0800
From:   Martin KaFai Lau <kafai@...com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        syzbot+0e91362d99386dc5de99@...kaller.appspotmail.com,
        netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for
 live packet mode

On Thu, Mar 10, 2022 at 11:56:20PM +0100, Toke Høiland-Jørgensen wrote:
> The live packet mode uses some extra space at the start of each page to
> cache data structures so they don't have to be rebuilt at every repetition.
> This space wasn't correctly accounted for in the size checking of the
> arguments supplied to userspace. In addition, the definition of the frame
> size should include the size of the skb_shared_info (as there is other
> logic that subtracts the size of this).
> 
> Together, these mistakes resulted in userspace being able to trip the
> XDP_WARN() in xdp_update_frame_from_buff(), which syzbot discovered in
> short order. Fix this by changing the frame size define and adding the
> extra headroom to the bpf_prog_test_run_xdp() function. Also drop the
> max_len parameter to the page_pool init, since this is related to DMA which
> is not used for the page pool instance in PROG_TEST_RUN.
> 
> Reported-by: syzbot+0e91362d99386dc5de99@...kaller.appspotmail.com
> Fixes: b530e9e1063e ("bpf: Add "live packet" mode for XDP in BPF_PROG_RUN")
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
>  net/bpf/test_run.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 24405a280a9b..e7b9c2636d10 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -112,8 +112,7 @@ struct xdp_test_data {
>  	u32 frame_cnt;
>  };
>  
> -#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head)	\
> -			     - sizeof(struct skb_shared_info))
> +#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head))
>  #define TEST_XDP_MAX_BATCH 256
>  
>  static void xdp_test_run_init_page(struct page *page, void *arg)
> @@ -156,7 +155,6 @@ static int xdp_test_run_setup(struct xdp_test_data *xdp, struct xdp_buff *orig_c
>  		.flags = 0,
>  		.pool_size = xdp->batch_size,
>  		.nid = NUMA_NO_NODE,
> -		.max_len = TEST_XDP_FRAME_SIZE,
>  		.init_callback = xdp_test_run_init_page,
>  		.init_arg = xdp,
>  	};
> @@ -1230,6 +1228,8 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>  			batch_size = NAPI_POLL_WEIGHT;
>  		else if (batch_size > TEST_XDP_MAX_BATCH)
>  			return -E2BIG;
> +
> +		headroom += sizeof(struct xdp_page_head);
The orig_ctx->data_end will ensure there is a sizeof(struct skb_shared_info)
tailroom ?  It is quite tricky to read but I don't have a better idea
either.

Acked-by: Martin KaFai Lau <kafai@...com>

>  	} else if (batch_size) {
>  		return -EINVAL;
>  	}
> -- 
> 2.35.1
>