diff mbox series

[net] tun: fix tun_napi_alloc_frags() frag allocator

Message ID 1518734835.3715.209.camel@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series [net] tun: fix tun_napi_alloc_frags() frag allocator | expand

Commit Message

Eric Dumazet Feb. 15, 2018, 10:47 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

<Mark Rutland reported>
    While fuzzing arm64 v4.16-rc1 with Syzkaller, I've been hitting a
    misaligned atomic in __skb_clone:

        atomic_inc(&(skb_shinfo(skb)->dataref));

   where dataref doesn't have the required natural alignment, and the
   atomic operation faults. e.g. i often see it aligned to a single
   byte boundary rather than a four byte boundary.

   AFAICT, the skb_shared_info is misaligned at the instant it's
   allocated in __napi_alloc_skb()  __napi_alloc_skb()
</end of report>

Problem is caused by tun_napi_alloc_frags() using
napi_alloc_frag() with user provided seg sizes,
leading to other users of this API getting unaligned
page fragments.

Since we would like to not necessarily add paddings or alignments to
the frags that tun_napi_alloc_frags() attaches to the skb, switch to
another page frag allocator.

As a bonus skb_page_frag_refill() can use GFP_KERNEL allocations,
meaning that we can not deplete memory reserves as easily.

Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
---
 drivers/net/tun.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Comments

David Miller Feb. 16, 2018, 9:21 p.m. UTC | #1 
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 15 Feb 2018 14:47:15 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> <Mark Rutland reported>
>     While fuzzing arm64 v4.16-rc1 with Syzkaller, I've been hitting a
>     misaligned atomic in __skb_clone:
> 
>         atomic_inc(&(skb_shinfo(skb)->dataref));
> 
>    where dataref doesn't have the required natural alignment, and the
>    atomic operation faults. e.g. i often see it aligned to a single
>    byte boundary rather than a four byte boundary.
> 
>    AFAICT, the skb_shared_info is misaligned at the instant it's
>    allocated in __napi_alloc_skb()  __napi_alloc_skb()
> </end of report>
> 
> Problem is caused by tun_napi_alloc_frags() using
> napi_alloc_frag() with user provided seg sizes,
> leading to other users of this API getting unaligned
> page fragments.
> 
> Since we would like to not necessarily add paddings or alignments to
> the frags that tun_napi_alloc_frags() attaches to the skb, switch to
> another page frag allocator.
> 
> As a bonus skb_page_frag_refill() can use GFP_KERNEL allocations,
> meaning that we can not deplete memory reserves as easily.
> 
> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Mark Rutland <mark.rutland@arm.com>

Applied and queued up for -stable, thanks Eric.
diff mbox series

Patch

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;