| 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: <CAHTyZGyAO_NtuNiH62m5z8tEWjjWCobsyr-wvSLfeBNaBEYe6Q@mail.gmail.com>
Date: Mon, 16 Dec 2024 15:59:12 +0100
From: ericnetdev dumazet <erdnetdev@...il.com>
To: Ido Schimmel <idosch@...dia.com>
Cc: Eric Dumazet <edumazet@...gle.com>, 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
Le lun. 16 déc. 2024 à 15:44, Ido Schimmel <idosch@...dia.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@...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/
Yes, I am referring to a generic test done in locations where a
malicious skb could be cooked/transformed.
Thanks !
Powered by blists - more mailing lists