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: <CANn89i+y77-1skcxeq+OAeOVBDXhgZb75yZCq8+NBpHtZGySmw@mail.gmail.com>
Date: Wed, 2 Oct 2024 09:27:55 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Daniel Yang <danielyangkang@...il.com>, Daniel Borkmann <daniel@...earbox.net>
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 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 ?