Thread (11 messages) 11 messages, 4 authors, 2023-08-04
STALE1031d

[PATCH v4 1/2] drivers: net: prevent tun_build_skb() to exceed the packet size limit

From: Andrew Kanner <hidden>
Date: 2023-08-01 22:09:28
Also in: lkml, netdev
Subsystem: networking drivers, the rest, tun/tap driver · Maintainers: Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Willem de Bruijn, Jason Wang

Using the syzkaller repro with reduced packet size it was discovered
that XDP_PACKET_HEADROOM is not checked in tun_can_build_skb(),
although pad may be incremented in tun_build_skb(). This may end up
with exceeding the PAGE_SIZE limit in tun_build_skb().

Fixes: 7df13219d757 ("tun: reserve extra headroom only when XDP is set")
Link: https://syzkaller.appspot.com/bug?extid=f817490f5bd20541b90a
Signed-off-by: Andrew Kanner <redacted>
---

Notes:
    v3 -> v4:
    * fall back to v1, fixing only missing XDP_PACKET_HEADROOM in pad and
      removing bpf_xdp_adjust_tail() check for frame_sz.
    * added rcu read lock, noted by Jason Wang [off-list ref] in v1
    * I decided to leave the packet length check in tun_can_build_skb()
      instead of moving to tun_build_skb() suggested by Jason Wang
      [off-list ref]. Otherwise extra packets will be dropped
      without falling back to tun_alloc_skb(). And in the discussion of v3
      Jesper Dangaard Brouer [off-list ref] noticed that XDP is ok
      with a higher order pages if it's a contiguous physical memory
      allocation, so falling to tun_alloc_skb() -> do_xdp_generic() should
      be ok.
    
    v2 -> v3:
    * attach the forgotten changelog
    
    v1 -> v2:
    * merged 2 patches in 1, fixing both issues: WARN_ON_ONCE with
      syzkaller repro and missing XDP_PACKET_HEADROOM in pad
    * changed the title and description of the execution path, suggested
      by Jason Wang [off-list ref]
    * move the limit check from tun_can_build_skb() to tun_build_skb() to
      remove duplication and locking issue, and also drop the packet in
      case of a failed check - noted by Jason Wang [off-list ref]

 drivers/net/tun.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d75456adc62a..a1d04bc9485f 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1582,6 +1582,9 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
 static bool tun_can_build_skb(struct tun_struct *tun, struct tun_file *tfile,
 			      int len, int noblock, bool zerocopy)
 {
+	struct bpf_prog *xdp_prog;
+	int pad = TUN_RX_PAD;
+
 	if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
 		return false;
 
@@ -1594,7 +1597,13 @@ static bool tun_can_build_skb(struct tun_struct *tun, struct tun_file *tfile,
 	if (zerocopy)
 		return false;
 
-	if (SKB_DATA_ALIGN(len + TUN_RX_PAD) +
+	rcu_read_lock();
+	xdp_prog = rcu_dereference(tun->xdp_prog);
+	if (xdp_prog)
+		pad += XDP_PACKET_HEADROOM;
+	rcu_read_unlock();
+
+	if (SKB_DATA_ALIGN(len + pad) +
 	    SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) > PAGE_SIZE)
 		return false;
 
-- 
2.39.3

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help