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, linux-kernel@vger.kernel.org
Cc: syzbot+f817490f5bd20541b90a@syzkaller.appspotmail.com,
linux-kernel-mentees@lists.linuxfoundation.org
Subject: [PATCH v3] drivers: net: prevent tun_get_user() to exceed xdp size limits
Date: Tue, 25 Jul 2023 18:54:04 +0300 [thread overview]
Message-ID: <20230725155403.796-1-andrew.kanner@gmail.com> (raw)
Syzkaller reported the following issue:
=======================================
Too BIG xdp->frame_sz = 131072
WARNING: CPU: 0 PID: 5020 at net/core/filter.c:4121
____bpf_xdp_adjust_tail net/core/filter.c:4121 [inline]
WARNING: CPU: 0 PID: 5020 at net/core/filter.c:4121
bpf_xdp_adjust_tail+0x466/0xa10 net/core/filter.c:4103
...
Call Trace:
<TASK>
bpf_prog_4add87e5301a4105+0x1a/0x1c
__bpf_prog_run include/linux/filter.h:600 [inline]
bpf_prog_run_xdp include/linux/filter.h:775 [inline]
bpf_prog_run_generic_xdp+0x57e/0x11e0 net/core/dev.c:4721
netif_receive_generic_xdp net/core/dev.c:4807 [inline]
do_xdp_generic+0x35c/0x770 net/core/dev.c:4866
tun_get_user+0x2340/0x3ca0 drivers/net/tun.c:1919
tun_chr_write_iter+0xe8/0x210 drivers/net/tun.c:2043
call_write_iter include/linux/fs.h:1871 [inline]
new_sync_write fs/read_write.c:491 [inline]
vfs_write+0x650/0xe40 fs/read_write.c:584
ksys_write+0x12f/0x250 fs/read_write.c:637
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
xdp->frame_sz > PAGE_SIZE check was introduced in commit c8741e2bfe87
("xdp: Allow bpf_xdp_adjust_tail() to grow packet size"). But
tun_get_user() still provides an execution path with do_xdp_generic()
and exceed XDP limits for packet size.
Using the syzkaller repro with reduced packet size it was also
discovered that XDP_PACKET_HEADROOM is not checked in
tun_can_build_skb(), although pad may be incremented in
tun_build_skb().
If we move the limit check from tun_can_build_skb() to tun_build_skb()
we will make xdp to be used only in tun_build_skb(), without falling
in tun_alloc_skb(), etc. And moreover we will drop the packet which
can't be processed in tun_build_skb().
Reported-and-tested-by: syzbot+f817490f5bd20541b90a@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/000000000000774b9205f1d8a80d@google.com/T/
Link: https://syzkaller.appspot.com/bug?id=5335c7c62bfff89bbb1c8f14cdabebe91909060f
Fixes: 7df13219d757 ("tun: reserve extra headroom only when XDP is set")
Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com>
---
Notes:
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 | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d75456adc62a..7c2b05ce0421 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1594,10 +1594,6 @@ 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) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) > PAGE_SIZE)
- return false;
-
return true;
}
@@ -1673,6 +1669,9 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
buflen += SKB_DATA_ALIGN(len + pad);
rcu_read_unlock();
+ if (buflen > PAGE_SIZE)
+ return ERR_PTR(-EFAULT);
+
alloc_frag->offset = ALIGN((u64)alloc_frag->offset, SMP_CACHE_BYTES);
if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, GFP_KERNEL)))
return ERR_PTR(-ENOMEM);
--
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, 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 v3] drivers: net: prevent tun_get_user() to exceed xdp size limits
Date: Tue, 25 Jul 2023 18:54:04 +0300 [thread overview]
Message-ID: <20230725155403.796-1-andrew.kanner@gmail.com> (raw)
Syzkaller reported the following issue:
=======================================
Too BIG xdp->frame_sz = 131072
WARNING: CPU: 0 PID: 5020 at net/core/filter.c:4121
____bpf_xdp_adjust_tail net/core/filter.c:4121 [inline]
WARNING: CPU: 0 PID: 5020 at net/core/filter.c:4121
bpf_xdp_adjust_tail+0x466/0xa10 net/core/filter.c:4103
...
Call Trace:
<TASK>
bpf_prog_4add87e5301a4105+0x1a/0x1c
__bpf_prog_run include/linux/filter.h:600 [inline]
bpf_prog_run_xdp include/linux/filter.h:775 [inline]
bpf_prog_run_generic_xdp+0x57e/0x11e0 net/core/dev.c:4721
netif_receive_generic_xdp net/core/dev.c:4807 [inline]
do_xdp_generic+0x35c/0x770 net/core/dev.c:4866
tun_get_user+0x2340/0x3ca0 drivers/net/tun.c:1919
tun_chr_write_iter+0xe8/0x210 drivers/net/tun.c:2043
call_write_iter include/linux/fs.h:1871 [inline]
new_sync_write fs/read_write.c:491 [inline]
vfs_write+0x650/0xe40 fs/read_write.c:584
ksys_write+0x12f/0x250 fs/read_write.c:637
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
xdp->frame_sz > PAGE_SIZE check was introduced in commit c8741e2bfe87
("xdp: Allow bpf_xdp_adjust_tail() to grow packet size"). But
tun_get_user() still provides an execution path with do_xdp_generic()
and exceed XDP limits for packet size.
Using the syzkaller repro with reduced packet size it was also
discovered that XDP_PACKET_HEADROOM is not checked in
tun_can_build_skb(), although pad may be incremented in
tun_build_skb().
If we move the limit check from tun_can_build_skb() to tun_build_skb()
we will make xdp to be used only in tun_build_skb(), without falling
in tun_alloc_skb(), etc. And moreover we will drop the packet which
can't be processed in tun_build_skb().
Reported-and-tested-by: syzbot+f817490f5bd20541b90a@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/000000000000774b9205f1d8a80d@google.com/T/
Link: https://syzkaller.appspot.com/bug?id=5335c7c62bfff89bbb1c8f14cdabebe91909060f
Fixes: 7df13219d757 ("tun: reserve extra headroom only when XDP is set")
Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com>
---
Notes:
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 | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d75456adc62a..7c2b05ce0421 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1594,10 +1594,6 @@ 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) +
- SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) > PAGE_SIZE)
- return false;
-
return true;
}
@@ -1673,6 +1669,9 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
buflen += SKB_DATA_ALIGN(len + pad);
rcu_read_unlock();
+ if (buflen > PAGE_SIZE)
+ return ERR_PTR(-EFAULT);
+
alloc_frag->offset = ALIGN((u64)alloc_frag->offset, SMP_CACHE_BYTES);
if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, GFP_KERNEL)))
return ERR_PTR(-ENOMEM);
--
2.39.3
next reply other threads:[~2023-07-25 15:54 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-25 15:54 Andrew Kanner [this message]
2023-07-25 15:54 ` [PATCH v3] drivers: net: prevent tun_get_user() to exceed xdp size limits Andrew Kanner
2023-07-26 2:09 ` Jason Wang
2023-07-26 2:09 ` Jason Wang
2023-07-26 7:20 ` Andrew Kanner
2023-07-26 7:20 ` Andrew Kanner
2023-07-26 9:02 ` Jesper Dangaard Brouer
2023-07-26 9:02 ` Jesper Dangaard Brouer
2023-07-26 19:37 ` David Ahern
2023-07-26 19:37 ` David Ahern
2023-07-27 0:27 ` David Ahern
2023-07-27 0:27 ` David Ahern
2023-07-27 6:07 ` Jason Wang
2023-07-27 6:07 ` Jason Wang
2023-07-27 9:30 ` Paolo Abeni
2023-07-27 9:30 ` Paolo Abeni
2023-07-27 11:13 ` Jesper Dangaard Brouer
2023-07-27 11:13 ` Jesper Dangaard Brouer
2023-07-27 23:48 ` Andrew Kanner
2023-07-27 23:48 ` Andrew Kanner
2023-07-28 0:11 ` David Ahern
2023-07-28 0:11 ` David Ahern
2023-07-31 15:47 ` Andrew Kanner
2023-07-31 15:47 ` Andrew Kanner
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=20230725155403.796-1-andrew.kanner@gmail.com \
--to=andrew.kanner@gmail.com \
--cc=brouer@redhat.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jasowang@redhat.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.