netdev - Re: v4.16-rc1 misaligned atomics in skb__clone / __napi_alloc

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 15 Feb 2018 17:57:00 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: v4.16-rc1 misaligned atomics in skb__clone / __napi_alloc_skb

On Thu, Feb 15, 2018 at 09:43:06AM -0800, Eric Dumazet wrote:
> On Thu, 2018-02-15 at 09:24 -0800, Eric Dumazet wrote:
> > 
> > I will send something more suited to original intent of these commits :
> > 
> > 90e33d45940793def6f773b2d528e9f3c84ffdc7 tun: enable napi_gro_frags()
> > for TUN/TAP driver
> > 943170998b200190f99d3fe7e771437e2c51f319 tun: enable NAPI for TUN/TAP driver
> 
> Can you try this patch ?

Looks good! No splats after 10 minutes with a test that usually fails in
a few seconds.

FWIW:

Tested-by: Mark Rutland <mark.rutland@....com>

Thanks,
Mark.

>  drivers/net/tun.c |   16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 81e6cc951e7fc7c983919365c34842c34bcaedcf..b52258c327d2e1d7c7476de345e49f082909c246 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1489,27 +1489,23 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
>  	skb->truesize += skb->data_len;
>  
>  	for (i = 1; i < it->nr_segs; i++) {
> +		struct page_frag *pfrag = &current->task_frag;
>  		size_t fragsz = it->iov[i].iov_len;
> -		unsigned long offset;
> -		struct page *page;
> -		void *data;
>  
>  		if (fragsz == 0 || fragsz > PAGE_SIZE) {
>  			err = -EINVAL;
>  			goto free;
>  		}
>  
> -		local_bh_disable();
> -		data = napi_alloc_frag(fragsz);
> -		local_bh_enable();
> -		if (!data) {
> +		if (!skb_page_frag_refill(fragsz, pfrag, GFP_KERNEL)) {
>  			err = -ENOMEM;
>  			goto free;
>  		}
>  
> -		page = virt_to_head_page(data);
> -		offset = data - page_address(page);
> -		skb_fill_page_desc(skb, i - 1, page, offset, fragsz);
> +		skb_fill_page_desc(skb, i - 1, pfrag->page,
> +				   pfrag->offset, fragsz);
> +		page_ref_inc(pfrag->page);
> +		pfrag->offset += fragsz;
>  	}
>  
>  	return skb;
>