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
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 to exceed XDP limits
for packet size. The way to mitigate this is to reduce xdp to be used
only in tun_build_skb(), without falling in tun_alloc_skb(), etc.
Reported-and-tested-by: [email protected]
Closes: https://lore.kernel.org/all/[email protected]/T/
Link: https://syzkaller.appspot.com/bug?id=5335c7c62bfff89bbb1c8f14cdabebe91909060f
Signed-off-by: Andrew Kanner <[email protected]>
---
drivers/net/tun.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d75456adc62a..18ccbbe9830a 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1580,8 +1580,14 @@ 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)
+ int len, int noblock, bool zerocopy, int *skb_xdp)
{
+ if (SKB_DATA_ALIGN(len + TUN_RX_PAD) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) > PAGE_SIZE) {
+ *skb_xdp = 0;
+ return false;
+ }
+
if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
return false;
@@ -1594,10 +1600,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;
}
@@ -1809,7 +1811,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
zerocopy = true;
}
- if (!frags && tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
+ if (tun_can_build_skb(tun, tfile, len, noblock, zerocopy, &skb_xdp) && !frags) {
/* For the packet that is not easy to be processed
* (e.g gso or jumbo packet), we will do it at after
* skb was created with generic XDP routine.
--
2.39.3
On Tue, Jul 25, 2023 at 6:14 AM Andrew Kanner <[email protected]> wrote:
>
> 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
>
> frame_sz < PAGE_SIZE check was introduced in commit c8741e2bfe87
> ("xdp: Allow bpf_xdp_adjust_tail() to grow packet size").
I wonder if this check makes sense for xdp generic when skb is linearized.
> But
> tun_get_user() still provides an execution path to exceed XDP limits
> for packet size.
I'd suggest being specific to the execution path, which is actually
the generic XDP.
> The way to mitigate this is to reduce xdp to be used
> only in tun_build_skb(), without falling in tun_alloc_skb(), etc.
>
> Reported-and-tested-by: [email protected]
> Closes: https://lore.kernel.org/all/[email protected]/T/
> Link: https://syzkaller.appspot.com/bug?id=5335c7c62bfff89bbb1c8f14cdabebe91909060f
> Signed-off-by: Andrew Kanner <[email protected]>
> ---
> drivers/net/tun.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index d75456adc62a..18ccbbe9830a 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1580,8 +1580,14 @@ 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)
> + int len, int noblock, bool zerocopy, int *skb_xdp)
> {
> + if (SKB_DATA_ALIGN(len + TUN_RX_PAD) +
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) > PAGE_SIZE) {
> + *skb_xdp = 0;
So if XDP is enabled, the packet will bypass the bpf program which is
sub-optimal. I think we should at least drop the packet in this case.
Thanks
> + return false;
> + }
> +
> if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
> return false;
>
> @@ -1594,10 +1600,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;
> }
>
> @@ -1809,7 +1811,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> zerocopy = true;
> }
>
> - if (!frags && tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
> + if (tun_can_build_skb(tun, tfile, len, noblock, zerocopy, &skb_xdp) && !frags) {
> /* For the packet that is not easy to be processed
> * (e.g gso or jumbo packet), we will do it at after
> * skb was created with generic XDP routine.
> --
> 2.39.3
>