From: Andrew Kanner <andrew.kanner@gmail.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, jasowang@redhat.com, netdev@vger.kernel.org,
brouer@redhat.com, dsahern@gmail.com, jbrouer@redhat.com,
john.fastabend@gmail.com, linux-kernel@vger.kernel.org
Cc: syzbot+f817490f5bd20541b90a@syzkaller.appspotmail.com,
linux-kernel-mentees@lists.linuxfoundation.org
Subject: [PATCH v4 1/2] drivers: net: prevent tun_build_skb() to exceed the packet size limit
Date: Wed, 2 Aug 2023 01:07:11 +0300 [thread overview]
Message-ID: <20230801220710.464-1-andrew.kanner@gmail.com> (raw)
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 <andrew.kanner@gmail.com>
---
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 <jasowang@redhat.com> 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
<jasowang@redhat.com>. Otherwise extra packets will be dropped
without falling back to tun_alloc_skb(). And in the discussion of v3
Jesper Dangaard Brouer <jbrouer@redhat.com> 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 <jasowang@redhat.com>
* 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 <jasowang@redhat.com>
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
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Kanner <andrew.kanner@gmail.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, jasowang@redhat.com, netdev@vger.kernel.org,
brouer@redhat.com, dsahern@gmail.com, jbrouer@redhat.com,
john.fastabend@gmail.com, linux-kernel@vger.kernel.org
Cc: linux-kernel-mentees@lists.linuxfoundation.org,
syzbot+f817490f5bd20541b90a@syzkaller.appspotmail.com,
Andrew Kanner <andrew.kanner@gmail.com>
Subject: [PATCH v4 1/2] drivers: net: prevent tun_build_skb() to exceed the packet size limit
Date: Wed, 2 Aug 2023 01:07:11 +0300 [thread overview]
Message-ID: <20230801220710.464-1-andrew.kanner@gmail.com> (raw)
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 <andrew.kanner@gmail.com>
---
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 <jasowang@redhat.com> 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
<jasowang@redhat.com>. Otherwise extra packets will be dropped
without falling back to tun_alloc_skb(). And in the discussion of v3
Jesper Dangaard Brouer <jbrouer@redhat.com> 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 <jasowang@redhat.com>
* 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 <jasowang@redhat.com>
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
next reply other threads:[~2023-08-01 22:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-01 22:07 Andrew Kanner [this message]
2023-08-01 22:07 ` [PATCH v4 1/2] drivers: net: prevent tun_build_skb() to exceed the packet size limit Andrew Kanner
2023-08-01 22:07 ` [PATCH v4 2/2] net: core: remove unnecessary frame_sz check in bpf_xdp_adjust_tail() Andrew Kanner
2023-08-01 22:07 ` Andrew Kanner
2023-08-02 13:56 ` Jesper Dangaard Brouer
2023-08-02 13:56 ` Jesper Dangaard Brouer
2023-08-03 3:20 ` Jason Wang
2023-08-03 3:20 ` Jason Wang
2023-08-03 18:06 ` Andrew Kanner
2023-08-03 18:06 ` Andrew Kanner
2023-08-02 1:07 ` [PATCH v4 1/2] drivers: net: prevent tun_build_skb() to exceed the packet size limit David Ahern
2023-08-02 1:07 ` David Ahern
2023-08-02 12:16 ` Andrew Kanner
2023-08-02 12:16 ` Andrew Kanner
2023-08-02 14:13 ` Jesper Dangaard Brouer
2023-08-02 14:13 ` Jesper Dangaard Brouer
2023-08-03 3:19 ` Jason Wang
2023-08-03 3:19 ` Jason Wang
2023-08-03 17:53 ` Andrew Kanner
2023-08-03 17:53 ` Andrew Kanner
2023-08-04 9:30 ` Jesper Dangaard Brouer
2023-08-04 9:30 ` Jesper Dangaard Brouer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230801220710.464-1-andrew.kanner@gmail.com \
--to=andrew.kanner@gmail.com \
--cc=brouer@redhat.com \
--cc=davem@davemloft.net \
--cc=dsahern@gmail.com \
--cc=edumazet@google.com \
--cc=jasowang@redhat.com \
--cc=jbrouer@redhat.com \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=syzbot+f817490f5bd20541b90a@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.