* [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit
@ 2024-12-16 13:42 Ido Schimmel
2024-12-16 13:48 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Ido Schimmel @ 2024-12-16 13:42 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, horms, gnault,
Ido Schimmel
The VXLAN driver does not verify that transmitted packets have an
Ethernet header in the linear part of the skb, which can result in the
driver accessing uninitialized memory while processing the Ethernet
header [1]. Issue can be reproduced using [2].
Fix by checking that we can pull the Ethernet header into the linear
part of the skb. Note that the driver can transmit IP packets, but this
is handled earlier in the xmit path.
[1]
CPU: 6 UID: 0 PID: 404 Comm: bpftool Tainted: G B 6.12.0-rc7-custom-g10d3437464d3 #232
Tainted: [B]=BAD_PAGE
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014
=====================================================
=====================================================
BUG: KMSAN: uninit-value in __vxlan_find_mac+0x449/0x450
__vxlan_find_mac+0x449/0x450
vxlan_xmit+0x1265/0x2f70
dev_hard_start_xmit+0x239/0x7e0
__dev_queue_xmit+0x2d65/0x45e0
__bpf_redirect+0x6d2/0xf60
bpf_clone_redirect+0x2c7/0x450
bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22
bpf_test_run+0x60f/0xca0
bpf_prog_test_run_skb+0x115d/0x2300
bpf_prog_test_run+0x3b3/0x5c0
__sys_bpf+0x501/0xc60
__x64_sys_bpf+0xa8/0xf0
do_syscall_64+0xd9/0x1b0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Uninit was stored to memory at:
__vxlan_find_mac+0x442/0x450
vxlan_xmit+0x1265/0x2f70
dev_hard_start_xmit+0x239/0x7e0
__dev_queue_xmit+0x2d65/0x45e0
__bpf_redirect+0x6d2/0xf60
bpf_clone_redirect+0x2c7/0x450
bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22
bpf_test_run+0x60f/0xca0
bpf_prog_test_run_skb+0x115d/0x2300
bpf_prog_test_run+0x3b3/0x5c0
__sys_bpf+0x501/0xc60
__x64_sys_bpf+0xa8/0xf0
do_syscall_64+0xd9/0x1b0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Uninit was created at:
kmem_cache_alloc_node_noprof+0x4a8/0x9e0
kmalloc_reserve+0xd1/0x420
pskb_expand_head+0x1b4/0x15f0
skb_ensure_writable+0x2ee/0x390
bpf_clone_redirect+0x16a/0x450
bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22
bpf_test_run+0x60f/0xca0
bpf_prog_test_run_skb+0x115d/0x2300
bpf_prog_test_run+0x3b3/0x5c0
__sys_bpf+0x501/0xc60
__x64_sys_bpf+0xa8/0xf0
do_syscall_64+0xd9/0x1b0
[2]
$ cat mac_repo.bpf.c
// SPDX-License-Identifier: GPL-2.0
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
SEC("lwt_xmit")
int mac_repo(struct __sk_buff *skb)
{
return bpf_clone_redirect(skb, 100, 0);
}
$ clang -O2 -target bpf -c mac_repo.bpf.c -o mac_repo.o
# ip link add name vx0 up index 100 type vxlan id 10010 dstport 4789 local 192.0.2.1
# bpftool prog load mac_repo.o /sys/fs/bpf/mac_repo
# echo -ne "\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41" | \
bpftool prog run pinned /sys/fs/bpf/mac_repo data_in - repeat 10
Fixes: d342894c5d2f ("vxlan: virtual extensible lan")
Reported-by: syzbot+35e7e2811bbe5777b20e@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6735d39a.050a0220.1324f8.0096.GAE@google.com/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
If this is accepted, I will change dev_core_stats_tx_dropped_inc() to
dev_dstats_tx_dropped() in net-next.
---
drivers/net/vxlan/vxlan_core.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 9ea63059d52d..4cbde7a88205 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2722,6 +2722,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
struct vxlan_dev *vxlan = netdev_priv(dev);
struct vxlan_rdst *rdst, *fdst = NULL;
const struct ip_tunnel_info *info;
+ enum skb_drop_reason reason;
struct vxlan_fdb *f;
struct ethhdr *eth;
__be32 vni = 0;
@@ -2746,6 +2747,15 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
}
}
+ reason = pskb_may_pull_reason(skb, ETH_HLEN);
+ if (unlikely(reason != SKB_NOT_DROPPED_YET)) {
+ dev_core_stats_tx_dropped_inc(dev);
+ vxlan_vnifilter_count(vxlan, vni, NULL,
+ VXLAN_VNI_STATS_TX_DROPS, 0);
+ kfree_skb_reason(skb, reason);
+ return NETDEV_TX_OK;
+ }
+
if (vxlan->cfg.flags & VXLAN_F_PROXY) {
eth = eth_hdr(skb);
if (ntohs(eth->h_proto) == ETH_P_ARP)
--
2.47.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit 2024-12-16 13:42 [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit Ido Schimmel @ 2024-12-16 13:48 ` Eric Dumazet 2024-12-16 14:44 ` Ido Schimmel 0 siblings, 1 reply; 5+ messages in thread From: Eric Dumazet @ 2024-12-16 13:48 UTC (permalink / raw) To: Ido Schimmel; +Cc: netdev, davem, kuba, pabeni, andrew+netdev, horms, gnault On Mon, Dec 16, 2024 at 2:43 PM Ido Schimmel <idosch@nvidia.com> wrote: > > The VXLAN driver does not verify that transmitted packets have an > Ethernet header in the linear part of the skb, which can result in the > driver accessing uninitialized memory while processing the Ethernet > header [1]. Issue can be reproduced using [2]. > > Fix by checking that we can pull the Ethernet header into the linear > part of the skb. Note that the driver can transmit IP packets, but this > is handled earlier in the xmit path. > > [1] > CPU: 6 UID: 0 PID: 404 Comm: bpftool Tainted: G B 6.12.0-rc7-custom-g10d3437464d3 #232 > Tainted: [B]=BAD_PAGE > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014 > ===================================================== > ===================================================== > BUG: KMSAN: uninit-value in __vxlan_find_mac+0x449/0x450 > __vxlan_find_mac+0x449/0x450 > vxlan_xmit+0x1265/0x2f70 > dev_hard_start_xmit+0x239/0x7e0 > __dev_queue_xmit+0x2d65/0x45e0 > __bpf_redirect+0x6d2/0xf60 > bpf_clone_redirect+0x2c7/0x450 > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > bpf_test_run+0x60f/0xca0 > bpf_prog_test_run_skb+0x115d/0x2300 > bpf_prog_test_run+0x3b3/0x5c0 > __sys_bpf+0x501/0xc60 > __x64_sys_bpf+0xa8/0xf0 > do_syscall_64+0xd9/0x1b0 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > Uninit was stored to memory at: > __vxlan_find_mac+0x442/0x450 > vxlan_xmit+0x1265/0x2f70 > dev_hard_start_xmit+0x239/0x7e0 > __dev_queue_xmit+0x2d65/0x45e0 > __bpf_redirect+0x6d2/0xf60 > bpf_clone_redirect+0x2c7/0x450 > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > bpf_test_run+0x60f/0xca0 > bpf_prog_test_run_skb+0x115d/0x2300 > bpf_prog_test_run+0x3b3/0x5c0 > __sys_bpf+0x501/0xc60 > __x64_sys_bpf+0xa8/0xf0 > do_syscall_64+0xd9/0x1b0 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > Uninit was created at: > kmem_cache_alloc_node_noprof+0x4a8/0x9e0 > kmalloc_reserve+0xd1/0x420 > pskb_expand_head+0x1b4/0x15f0 > skb_ensure_writable+0x2ee/0x390 > bpf_clone_redirect+0x16a/0x450 > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > bpf_test_run+0x60f/0xca0 > bpf_prog_test_run_skb+0x115d/0x2300 > bpf_prog_test_run+0x3b3/0x5c0 > __sys_bpf+0x501/0xc60 > __x64_sys_bpf+0xa8/0xf0 > do_syscall_64+0xd9/0x1b0 > > [2] > $ cat mac_repo.bpf.c > // SPDX-License-Identifier: GPL-2.0 > #include <linux/bpf.h> > #include <bpf/bpf_helpers.h> > > SEC("lwt_xmit") > int mac_repo(struct __sk_buff *skb) > { > return bpf_clone_redirect(skb, 100, 0); > } > > $ clang -O2 -target bpf -c mac_repo.bpf.c -o mac_repo.o > > # ip link add name vx0 up index 100 type vxlan id 10010 dstport 4789 local 192.0.2.1 > > # bpftool prog load mac_repo.o /sys/fs/bpf/mac_repo > > # echo -ne "\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41" | \ > bpftool prog run pinned /sys/fs/bpf/mac_repo data_in - repeat 10 > > Fixes: d342894c5d2f ("vxlan: virtual extensible lan") > Reported-by: syzbot+35e7e2811bbe5777b20e@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/netdev/6735d39a.050a0220.1324f8.0096.GAE@google.com/ > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > --- > If this is accepted, I will change dev_core_stats_tx_dropped_inc() to > dev_dstats_tx_dropped() in net-next. > --- > drivers/net/vxlan/vxlan_core.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c > index 9ea63059d52d..4cbde7a88205 100644 > --- a/drivers/net/vxlan/vxlan_core.c > +++ b/drivers/net/vxlan/vxlan_core.c > @@ -2722,6 +2722,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > struct vxlan_dev *vxlan = netdev_priv(dev); > struct vxlan_rdst *rdst, *fdst = NULL; > const struct ip_tunnel_info *info; > + enum skb_drop_reason reason; > struct vxlan_fdb *f; > struct ethhdr *eth; > __be32 vni = 0; > @@ -2746,6 +2747,15 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > } > } > > + reason = pskb_may_pull_reason(skb, ETH_HLEN); > + if (unlikely(reason != SKB_NOT_DROPPED_YET)) { > + dev_core_stats_tx_dropped_inc(dev); > + vxlan_vnifilter_count(vxlan, vni, NULL, > + VXLAN_VNI_STATS_TX_DROPS, 0); > + kfree_skb_reason(skb, reason); > + return NETDEV_TX_OK; > + } I think the plan was to use dev->min_header_len, in the generic part of networking stack, instead of having to copy/paste this code in all drivers. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit 2024-12-16 13:48 ` Eric Dumazet @ 2024-12-16 14:44 ` Ido Schimmel 2024-12-16 14:59 ` ericnetdev dumazet 0 siblings, 1 reply; 5+ messages in thread From: Ido Schimmel @ 2024-12-16 14:44 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, davem, kuba, pabeni, andrew+netdev, horms, gnault On Mon, Dec 16, 2024 at 02:48:04PM +0100, Eric Dumazet wrote: > On Mon, Dec 16, 2024 at 2:43 PM Ido Schimmel <idosch@nvidia.com> wrote: > > > > The VXLAN driver does not verify that transmitted packets have an > > Ethernet header in the linear part of the skb, which can result in the > > driver accessing uninitialized memory while processing the Ethernet > > header [1]. Issue can be reproduced using [2]. > > > > Fix by checking that we can pull the Ethernet header into the linear > > part of the skb. Note that the driver can transmit IP packets, but this > > is handled earlier in the xmit path. > > > > [1] > > CPU: 6 UID: 0 PID: 404 Comm: bpftool Tainted: G B 6.12.0-rc7-custom-g10d3437464d3 #232 > > Tainted: [B]=BAD_PAGE > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014 > > ===================================================== > > ===================================================== > > BUG: KMSAN: uninit-value in __vxlan_find_mac+0x449/0x450 > > __vxlan_find_mac+0x449/0x450 > > vxlan_xmit+0x1265/0x2f70 > > dev_hard_start_xmit+0x239/0x7e0 > > __dev_queue_xmit+0x2d65/0x45e0 > > __bpf_redirect+0x6d2/0xf60 > > bpf_clone_redirect+0x2c7/0x450 > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > bpf_test_run+0x60f/0xca0 > > bpf_prog_test_run_skb+0x115d/0x2300 > > bpf_prog_test_run+0x3b3/0x5c0 > > __sys_bpf+0x501/0xc60 > > __x64_sys_bpf+0xa8/0xf0 > > do_syscall_64+0xd9/0x1b0 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > Uninit was stored to memory at: > > __vxlan_find_mac+0x442/0x450 > > vxlan_xmit+0x1265/0x2f70 > > dev_hard_start_xmit+0x239/0x7e0 > > __dev_queue_xmit+0x2d65/0x45e0 > > __bpf_redirect+0x6d2/0xf60 > > bpf_clone_redirect+0x2c7/0x450 > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > bpf_test_run+0x60f/0xca0 > > bpf_prog_test_run_skb+0x115d/0x2300 > > bpf_prog_test_run+0x3b3/0x5c0 > > __sys_bpf+0x501/0xc60 > > __x64_sys_bpf+0xa8/0xf0 > > do_syscall_64+0xd9/0x1b0 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > Uninit was created at: > > kmem_cache_alloc_node_noprof+0x4a8/0x9e0 > > kmalloc_reserve+0xd1/0x420 > > pskb_expand_head+0x1b4/0x15f0 > > skb_ensure_writable+0x2ee/0x390 > > bpf_clone_redirect+0x16a/0x450 > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > bpf_test_run+0x60f/0xca0 > > bpf_prog_test_run_skb+0x115d/0x2300 > > bpf_prog_test_run+0x3b3/0x5c0 > > __sys_bpf+0x501/0xc60 > > __x64_sys_bpf+0xa8/0xf0 > > do_syscall_64+0xd9/0x1b0 > > > > [2] > > $ cat mac_repo.bpf.c > > // SPDX-License-Identifier: GPL-2.0 > > #include <linux/bpf.h> > > #include <bpf/bpf_helpers.h> > > > > SEC("lwt_xmit") > > int mac_repo(struct __sk_buff *skb) > > { > > return bpf_clone_redirect(skb, 100, 0); > > } > > > > $ clang -O2 -target bpf -c mac_repo.bpf.c -o mac_repo.o > > > > # ip link add name vx0 up index 100 type vxlan id 10010 dstport 4789 local 192.0.2.1 > > > > # bpftool prog load mac_repo.o /sys/fs/bpf/mac_repo > > > > # echo -ne "\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41" | \ > > bpftool prog run pinned /sys/fs/bpf/mac_repo data_in - repeat 10 > > > > Fixes: d342894c5d2f ("vxlan: virtual extensible lan") > > Reported-by: syzbot+35e7e2811bbe5777b20e@syzkaller.appspotmail.com > > Closes: https://lore.kernel.org/netdev/6735d39a.050a0220.1324f8.0096.GAE@google.com/ > > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > > --- > > If this is accepted, I will change dev_core_stats_tx_dropped_inc() to > > dev_dstats_tx_dropped() in net-next. > > --- > > drivers/net/vxlan/vxlan_core.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c > > index 9ea63059d52d..4cbde7a88205 100644 > > --- a/drivers/net/vxlan/vxlan_core.c > > +++ b/drivers/net/vxlan/vxlan_core.c > > @@ -2722,6 +2722,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > > struct vxlan_dev *vxlan = netdev_priv(dev); > > struct vxlan_rdst *rdst, *fdst = NULL; > > const struct ip_tunnel_info *info; > > + enum skb_drop_reason reason; > > struct vxlan_fdb *f; > > struct ethhdr *eth; > > __be32 vni = 0; > > @@ -2746,6 +2747,15 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > > } > > } > > > > + reason = pskb_may_pull_reason(skb, ETH_HLEN); > > + if (unlikely(reason != SKB_NOT_DROPPED_YET)) { > > + dev_core_stats_tx_dropped_inc(dev); > > + vxlan_vnifilter_count(vxlan, vni, NULL, > > + VXLAN_VNI_STATS_TX_DROPS, 0); > > + kfree_skb_reason(skb, reason); > > + return NETDEV_TX_OK; > > + } > > I think the plan was to use dev->min_header_len, in the generic part > of networking stack, > instead of having to copy/paste this code in all drivers. Are you referring to [1]? Tested it using the reproducer I mentioned and it seems to work. Is it still blocked by the empty_skb test? [1] https://lore.kernel.org/netdev/20240322122407.1329861-1-edumazet@google.com/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit 2024-12-16 14:44 ` Ido Schimmel @ 2024-12-16 14:59 ` ericnetdev dumazet 2024-12-16 15:21 ` Ido Schimmel 0 siblings, 1 reply; 5+ messages in thread From: ericnetdev dumazet @ 2024-12-16 14:59 UTC (permalink / raw) To: Ido Schimmel Cc: Eric Dumazet, netdev, davem, kuba, pabeni, andrew+netdev, horms, gnault Le lun. 16 déc. 2024 à 15:44, Ido Schimmel <idosch@nvidia.com> a écrit : > > On Mon, Dec 16, 2024 at 02:48:04PM +0100, Eric Dumazet wrote: > > On Mon, Dec 16, 2024 at 2:43 PM Ido Schimmel <idosch@nvidia.com> wrote: > > > > > > The VXLAN driver does not verify that transmitted packets have an > > > Ethernet header in the linear part of the skb, which can result in the > > > driver accessing uninitialized memory while processing the Ethernet > > > header [1]. Issue can be reproduced using [2]. > > > > > > Fix by checking that we can pull the Ethernet header into the linear > > > part of the skb. Note that the driver can transmit IP packets, but this > > > is handled earlier in the xmit path. > > > > > > [1] > > > CPU: 6 UID: 0 PID: 404 Comm: bpftool Tainted: G B 6.12.0-rc7-custom-g10d3437464d3 #232 > > > Tainted: [B]=BAD_PAGE > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014 > > > ===================================================== > > > ===================================================== > > > BUG: KMSAN: uninit-value in __vxlan_find_mac+0x449/0x450 > > > __vxlan_find_mac+0x449/0x450 > > > vxlan_xmit+0x1265/0x2f70 > > > dev_hard_start_xmit+0x239/0x7e0 > > > __dev_queue_xmit+0x2d65/0x45e0 > > > __bpf_redirect+0x6d2/0xf60 > > > bpf_clone_redirect+0x2c7/0x450 > > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > > bpf_test_run+0x60f/0xca0 > > > bpf_prog_test_run_skb+0x115d/0x2300 > > > bpf_prog_test_run+0x3b3/0x5c0 > > > __sys_bpf+0x501/0xc60 > > > __x64_sys_bpf+0xa8/0xf0 > > > do_syscall_64+0xd9/0x1b0 > > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > > > Uninit was stored to memory at: > > > __vxlan_find_mac+0x442/0x450 > > > vxlan_xmit+0x1265/0x2f70 > > > dev_hard_start_xmit+0x239/0x7e0 > > > __dev_queue_xmit+0x2d65/0x45e0 > > > __bpf_redirect+0x6d2/0xf60 > > > bpf_clone_redirect+0x2c7/0x450 > > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > > bpf_test_run+0x60f/0xca0 > > > bpf_prog_test_run_skb+0x115d/0x2300 > > > bpf_prog_test_run+0x3b3/0x5c0 > > > __sys_bpf+0x501/0xc60 > > > __x64_sys_bpf+0xa8/0xf0 > > > do_syscall_64+0xd9/0x1b0 > > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > > > Uninit was created at: > > > kmem_cache_alloc_node_noprof+0x4a8/0x9e0 > > > kmalloc_reserve+0xd1/0x420 > > > pskb_expand_head+0x1b4/0x15f0 > > > skb_ensure_writable+0x2ee/0x390 > > > bpf_clone_redirect+0x16a/0x450 > > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > > bpf_test_run+0x60f/0xca0 > > > bpf_prog_test_run_skb+0x115d/0x2300 > > > bpf_prog_test_run+0x3b3/0x5c0 > > > __sys_bpf+0x501/0xc60 > > > __x64_sys_bpf+0xa8/0xf0 > > > do_syscall_64+0xd9/0x1b0 > > > > > > [2] > > > $ cat mac_repo.bpf.c > > > // SPDX-License-Identifier: GPL-2.0 > > > #include <linux/bpf.h> > > > #include <bpf/bpf_helpers.h> > > > > > > SEC("lwt_xmit") > > > int mac_repo(struct __sk_buff *skb) > > > { > > > return bpf_clone_redirect(skb, 100, 0); > > > } > > > > > > $ clang -O2 -target bpf -c mac_repo.bpf.c -o mac_repo.o > > > > > > # ip link add name vx0 up index 100 type vxlan id 10010 dstport 4789 local 192.0.2.1 > > > > > > # bpftool prog load mac_repo.o /sys/fs/bpf/mac_repo > > > > > > # echo -ne "\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41" | \ > > > bpftool prog run pinned /sys/fs/bpf/mac_repo data_in - repeat 10 > > > > > > Fixes: d342894c5d2f ("vxlan: virtual extensible lan") > > > Reported-by: syzbot+35e7e2811bbe5777b20e@syzkaller.appspotmail.com > > > Closes: https://lore.kernel.org/netdev/6735d39a.050a0220.1324f8.0096.GAE@google.com/ > > > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > > > --- > > > If this is accepted, I will change dev_core_stats_tx_dropped_inc() to > > > dev_dstats_tx_dropped() in net-next. > > > --- > > > drivers/net/vxlan/vxlan_core.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c > > > index 9ea63059d52d..4cbde7a88205 100644 > > > --- a/drivers/net/vxlan/vxlan_core.c > > > +++ b/drivers/net/vxlan/vxlan_core.c > > > @@ -2722,6 +2722,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > > > struct vxlan_dev *vxlan = netdev_priv(dev); > > > struct vxlan_rdst *rdst, *fdst = NULL; > > > const struct ip_tunnel_info *info; > > > + enum skb_drop_reason reason; > > > struct vxlan_fdb *f; > > > struct ethhdr *eth; > > > __be32 vni = 0; > > > @@ -2746,6 +2747,15 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > > > } > > > } > > > > > > + reason = pskb_may_pull_reason(skb, ETH_HLEN); > > > + if (unlikely(reason != SKB_NOT_DROPPED_YET)) { > > > + dev_core_stats_tx_dropped_inc(dev); > > > + vxlan_vnifilter_count(vxlan, vni, NULL, > > > + VXLAN_VNI_STATS_TX_DROPS, 0); > > > + kfree_skb_reason(skb, reason); > > > + return NETDEV_TX_OK; > > > + } > > > > I think the plan was to use dev->min_header_len, in the generic part > > of networking stack, > > instead of having to copy/paste this code in all drivers. > > Are you referring to [1]? Tested it using the reproducer I mentioned and > it seems to work. Is it still blocked by the empty_skb test? > > [1] https://lore.kernel.org/netdev/20240322122407.1329861-1-edumazet@google.com/ Yes, I am referring to a generic test done in locations where a malicious skb could be cooked/transformed. Thanks ! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit 2024-12-16 14:59 ` ericnetdev dumazet @ 2024-12-16 15:21 ` Ido Schimmel 0 siblings, 0 replies; 5+ messages in thread From: Ido Schimmel @ 2024-12-16 15:21 UTC (permalink / raw) To: ericnetdev dumazet, daniel, sdf Cc: Eric Dumazet, netdev, davem, kuba, pabeni, andrew+netdev, horms, gnault On Mon, Dec 16, 2024 at 03:59:12PM +0100, ericnetdev dumazet wrote: > Le lun. 16 déc. 2024 à 15:44, Ido Schimmel <idosch@nvidia.com> a écrit : > > > > On Mon, Dec 16, 2024 at 02:48:04PM +0100, Eric Dumazet wrote: > > > On Mon, Dec 16, 2024 at 2:43 PM Ido Schimmel <idosch@nvidia.com> wrote: > > > > > > > > The VXLAN driver does not verify that transmitted packets have an > > > > Ethernet header in the linear part of the skb, which can result in the > > > > driver accessing uninitialized memory while processing the Ethernet > > > > header [1]. Issue can be reproduced using [2]. > > > > > > > > Fix by checking that we can pull the Ethernet header into the linear > > > > part of the skb. Note that the driver can transmit IP packets, but this > > > > is handled earlier in the xmit path. > > > > > > > > [1] > > > > CPU: 6 UID: 0 PID: 404 Comm: bpftool Tainted: G B 6.12.0-rc7-custom-g10d3437464d3 #232 > > > > Tainted: [B]=BAD_PAGE > > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014 > > > > ===================================================== > > > > ===================================================== > > > > BUG: KMSAN: uninit-value in __vxlan_find_mac+0x449/0x450 > > > > __vxlan_find_mac+0x449/0x450 > > > > vxlan_xmit+0x1265/0x2f70 > > > > dev_hard_start_xmit+0x239/0x7e0 > > > > __dev_queue_xmit+0x2d65/0x45e0 > > > > __bpf_redirect+0x6d2/0xf60 > > > > bpf_clone_redirect+0x2c7/0x450 > > > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > > > bpf_test_run+0x60f/0xca0 > > > > bpf_prog_test_run_skb+0x115d/0x2300 > > > > bpf_prog_test_run+0x3b3/0x5c0 > > > > __sys_bpf+0x501/0xc60 > > > > __x64_sys_bpf+0xa8/0xf0 > > > > do_syscall_64+0xd9/0x1b0 > > > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > > > > > Uninit was stored to memory at: > > > > __vxlan_find_mac+0x442/0x450 > > > > vxlan_xmit+0x1265/0x2f70 > > > > dev_hard_start_xmit+0x239/0x7e0 > > > > __dev_queue_xmit+0x2d65/0x45e0 > > > > __bpf_redirect+0x6d2/0xf60 > > > > bpf_clone_redirect+0x2c7/0x450 > > > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > > > bpf_test_run+0x60f/0xca0 > > > > bpf_prog_test_run_skb+0x115d/0x2300 > > > > bpf_prog_test_run+0x3b3/0x5c0 > > > > __sys_bpf+0x501/0xc60 > > > > __x64_sys_bpf+0xa8/0xf0 > > > > do_syscall_64+0xd9/0x1b0 > > > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > > > > > > > Uninit was created at: > > > > kmem_cache_alloc_node_noprof+0x4a8/0x9e0 > > > > kmalloc_reserve+0xd1/0x420 > > > > pskb_expand_head+0x1b4/0x15f0 > > > > skb_ensure_writable+0x2ee/0x390 > > > > bpf_clone_redirect+0x16a/0x450 > > > > bpf_prog_7423975f9f8be99f_mac_repo+0x20/0x22 > > > > bpf_test_run+0x60f/0xca0 > > > > bpf_prog_test_run_skb+0x115d/0x2300 > > > > bpf_prog_test_run+0x3b3/0x5c0 > > > > __sys_bpf+0x501/0xc60 > > > > __x64_sys_bpf+0xa8/0xf0 > > > > do_syscall_64+0xd9/0x1b0 > > > > > > > > [2] > > > > $ cat mac_repo.bpf.c > > > > // SPDX-License-Identifier: GPL-2.0 > > > > #include <linux/bpf.h> > > > > #include <bpf/bpf_helpers.h> > > > > > > > > SEC("lwt_xmit") > > > > int mac_repo(struct __sk_buff *skb) > > > > { > > > > return bpf_clone_redirect(skb, 100, 0); > > > > } > > > > > > > > $ clang -O2 -target bpf -c mac_repo.bpf.c -o mac_repo.o > > > > > > > > # ip link add name vx0 up index 100 type vxlan id 10010 dstport 4789 local 192.0.2.1 > > > > > > > > # bpftool prog load mac_repo.o /sys/fs/bpf/mac_repo > > > > > > > > # echo -ne "\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41\x41" | \ > > > > bpftool prog run pinned /sys/fs/bpf/mac_repo data_in - repeat 10 > > > > > > > > Fixes: d342894c5d2f ("vxlan: virtual extensible lan") > > > > Reported-by: syzbot+35e7e2811bbe5777b20e@syzkaller.appspotmail.com > > > > Closes: https://lore.kernel.org/netdev/6735d39a.050a0220.1324f8.0096.GAE@google.com/ > > > > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > > > > --- > > > > If this is accepted, I will change dev_core_stats_tx_dropped_inc() to > > > > dev_dstats_tx_dropped() in net-next. > > > > --- > > > > drivers/net/vxlan/vxlan_core.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c > > > > index 9ea63059d52d..4cbde7a88205 100644 > > > > --- a/drivers/net/vxlan/vxlan_core.c > > > > +++ b/drivers/net/vxlan/vxlan_core.c > > > > @@ -2722,6 +2722,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > > > > struct vxlan_dev *vxlan = netdev_priv(dev); > > > > struct vxlan_rdst *rdst, *fdst = NULL; > > > > const struct ip_tunnel_info *info; > > > > + enum skb_drop_reason reason; > > > > struct vxlan_fdb *f; > > > > struct ethhdr *eth; > > > > __be32 vni = 0; > > > > @@ -2746,6 +2747,15 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) > > > > } > > > > } > > > > > > > > + reason = pskb_may_pull_reason(skb, ETH_HLEN); > > > > + if (unlikely(reason != SKB_NOT_DROPPED_YET)) { > > > > + dev_core_stats_tx_dropped_inc(dev); > > > > + vxlan_vnifilter_count(vxlan, vni, NULL, > > > > + VXLAN_VNI_STATS_TX_DROPS, 0); > > > > + kfree_skb_reason(skb, reason); > > > > + return NETDEV_TX_OK; > > > > + } > > > > > > I think the plan was to use dev->min_header_len, in the generic part > > > of networking stack, > > > instead of having to copy/paste this code in all drivers. > > > > Are you referring to [1]? Tested it using the reproducer I mentioned and > > it seems to work. Is it still blocked by the empty_skb test? > > > > [1] https://lore.kernel.org/netdev/20240322122407.1329861-1-edumazet@google.com/ > > Yes, I am referring to a generic test done in locations where a > malicious skb could be cooked/transformed. OK, thanks Eric. I agree it's a better approach. Looks like it will allow us to revert 8bd67ebb50c0 ("net: bridge: xmit: make sure we have at least eth header len bytes") and other patches I might have missed. Daniel / Stan, are you guys still planning to adjust the empty_skb test so that Eric's patch could be applied without failing the CI? ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-12-16 15:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-16 13:42 [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit Ido Schimmel 2024-12-16 13:48 ` Eric Dumazet 2024-12-16 14:44 ` Ido Schimmel 2024-12-16 14:59 ` ericnetdev dumazet 2024-12-16 15:21 ` Ido Schimmel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).