netdev - Re: [PATCH net] vxlan: Avoid accessing uninitialized memory during xmit

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iLt9uKxzcceP=xWp5gr+VmghsZROwjHtK=878zDQ+7BpA@mail.gmail.com>
Date: Mon, 16 Dec 2024 14:48:04 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Ido Schimmel <idosch@...dia.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 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.