Re: [PATCH net v1] net: Fix tuntap uninitialized value

[Willem de Bruijn]

Jiayuan Chen wrote:
> Then tun/tap allocates an skb, it additionally allocates a prepad size
> (usually equal to NET_SKB_PAD) but leaves it uninitialized.
> 
> bpf_xdp_adjust_head() may move skb->data forward, which may lead to an
> issue.
> 
> Since the linear address is likely to be allocated from kmem_cache, it's
> unlikely to trigger a KMSAN warning. We need some tricks, such as forcing
> kmem_cache_shrink in __do_kmalloc_node, to reproduce the issue and trigger
> a KMSAN warning.
> 
> Reported-by: syzbot+0e6ddb1ef80986bdfe64@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/all/00000000000067f65105edbd295d@xxxxxxxxxx/T/
> Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
> ---
>  drivers/net/tun.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index f75f912a0225..111f83668b5e 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1463,6 +1463,7 @@ static struct sk_buff *tun_alloc_skb(struct tun_file *tfile,
>  	if (!skb)
>  		return ERR_PTR(err);
>  
> +	memset(skb->data, 0, prepad);
>  	skb_reserve(skb, prepad);
>  	skb_put(skb, linear);
>  	skb->data_len = len - linear;

Is this specific to the tun device?

This happens in generic (skb) xdp.

The stackdump shows a napi poll call stack

    bpf_prog_run_generic_xdp+0x13ff/0x1a30 net/core/dev.c:4782
    netif_receive_generic_xdp+0x639/0x910 net/core/dev.c:4845
    do_xdp_generic net/core/dev.c:4904 [inline]
    __netif_receive_skb_core+0x290f/0x6360 net/core/dev.c:5310
    __netif_receive_skb_one_core net/core/dev.c:5487 [inline]
    __netif_receive_skb+0xc8/0x5d0 net/core/dev.c:5603
    process_backlog+0x45a/0x890 net/core/dev.c:5931

Since this is syzbot, the skb will have come from a tun device,
seemingly with IFF_NAPI, and maybe IFF_NAPI_FRAGS.

But relevant to bpf_xdp_adjust_head is how the xdp metadata
was setup with xdp_prepare_buff, which here is called from
bpf_prog_run_generic_xdp:

        /* The XDP program wants to see the packet starting at the MAC
         * header.
         */
        mac_len = skb->data - skb_mac_header(skb);
        hard_start = skb->data - skb_headroom(skb);

        /* SKB "head" area always have tailroom for skb_shared_info */
        frame_sz = (void *)skb_end_pointer(skb) - hard_start;
        frame_sz += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

        rxqueue = netif_get_rxqueue(skb);
        xdp_init_buff(xdp, frame_sz, &rxqueue->xdp_rxq);
        xdp_prepare_buff(xdp, hard_start, skb_headroom(skb) - mac_len,
                         skb_headlen(skb) + mac_len, true);

> @@ -1621,6 +1622,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
>  		return ERR_PTR(-ENOMEM);
>  
>  	buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> +	memset(buf, 0, pad);
>  	copied = copy_page_from_iter(alloc_frag->page,
>  				     alloc_frag->offset + pad,
>  				     len, from);
> -- 
> 2.47.1
>