netdev - Re: [PATCH] Fix KMSAN infoleak, initialize unused data in pskb_expand

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: <ff22de41-07a5-4d16-9453-183b0c6a2872@iogearbox.net>
Date: Wed, 2 Oct 2024 13:55:31 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Eric Dumazet <edumazet@...gle.com>, Daniel Yang <danielyangkang@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 syzbot+346474e3bf0b26bd3090@...kaller.appspotmail.com
Subject: Re: [PATCH] Fix KMSAN infoleak, initialize unused data in
 pskb_expand_head

On 10/2/24 9:27 AM, Eric Dumazet wrote:
> On Wed, Oct 2, 2024 at 7:39 AM Daniel Yang <danielyangkang@...il.com> wrote:
>> pskb_expand_head doesn't initialize extra nhead bytes in header and
>> tail bytes, leading to KMSAN infoleak error. Fix by initializing data to
>> 0 with memset.
>>
>> Reported-by: syzbot+346474e3bf0b26bd3090@...kaller.appspotmail.com
>> Tested-by: Daniel Yang <danielyangkang@...il.com>
>> Signed-off-by: Daniel Yang <danielyangkang@...il.com>
> No no no.
>
> Please fix the root cause, instead of making slow all the users that
> got this right.
>
> Uninit was stored to memory at:
>   eth_header_parse+0xb8/0x110 net/ethernet/eth.c:204
>   dev_parse_header include/linux/netdevice.h:3158 [inline]
>   packet_rcv+0xefc/0x2050 net/packet/af_packet.c:2253
>   dev_queue_xmit_nit+0x114b/0x12a0 net/core/dev.c:2347
>   xmit_one net/core/dev.c:3584 [inline]
>   dev_hard_start_xmit+0x17d/0xa20 net/core/dev.c:3604
>   __dev_queue_xmit+0x3576/0x55e0 net/core/dev.c:4424
>   dev_queue_xmit include/linux/netdevice.h:3094 [inline]
>
>
> Sanity check [1] in __bpf_redirect_common() does not really help, if
> skb->len == 1 :/
>
> /* Verify that a link layer header is carried */
> if (unlikely(skb->mac_header >= skb->network_header || skb->len == 0)) {
>       kfree_skb(skb);
>       return -ERANGE;
> }
>
> These bugs keep showing up.
>
> [1]
>
> commit 114039b342014680911c35bd6b72624180fd669a
> Author: Stanislav Fomichev <sdf@...ichev.me>
> Date:   Mon Nov 21 10:03:39 2022 -0800
>
>      bpf: Move skb->len == 0 checks into __bpf_redirect
>
>      To avoid potentially breaking existing users.
>
>      Both mac/no-mac cases have to be amended; mac_header >= network_header
>      is not enough (verified with a new test, see next patch).
>
>      Fixes: fd1894224407 ("bpf: Don't redirect packets with invalid pkt_len")
>      Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
>      Link: https://lore.kernel.org/r/20221121180340.1983627-1-sdf@google.com
>      Signed-off-by: Martin KaFai Lau <martin.lau@...nel.org>
>
> I sent an earlier patch, this went nowhere I am afraid.
>
> https://www.spinics.net/lists/netdev/msg982652.html
>
> Daniel, can you take a look and fix this in net/core/filter.c ?

Ack, I'll have it on my todo list. Thanks!