| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z2A8vwvlKMqQf8jQ@shredder>
Date: Mon, 16 Dec 2024 16:44:15 +0200
From: Ido Schimmel <idosch@...dia.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
gnault@...hat.com
Subject: Re: [PATCH net] vxlan: Avoid accessing uninitialized memory during
xmit
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@...dia.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@...kaller.appspotmail.com
> > Closes: https://lore.kernel.org/netdev/6735d39a.050a0220.1324f8.0096.GAE@google.com/
> > Signed-off-by: Ido Schimmel <idosch@...dia.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/
Powered by blists - more mailing lists