Re: [PATCH net] Drop packets with invalid headers to prevent KMSAN infoleak

[Martin KaFai Lau]

On 10/21/24 6:37 PM, Daniel Yang wrote:
> > A test in selftests/bpf is needed to reproduce and better understand this.
> I don't know much about self tests but I've just been using the syzbot
> repro and #syz test at the link in the patch:
> https://syzkaller.appspot.com/bug?extid=346474e3bf0b26bd3090. Testing
> the patch showed that the uninitialized memory was not getting written
> to memory.
>
> > Only bpf_clone_redirect() is needed to reproduce or other bpf_skb_*() helpers calls
> > are needed to reproduce?

If only bpf_clone_redirect() is needed, it should be simple to write a selftest 
to reproduce it. It also helps to catch future regression.

Please tag the next respin as "bpf" also.

>  From what I can see in the crash report here:
> https://syzkaller.appspot.com/text?tag=CrashReport&x=10ba3ca9980000,
> only bpf_clone_redirect() is needed to trigger this issue. The issue
> seems to be that bpf_try_make_head_writable clones the skb and creates
> uninitialized memory but __bpf_tx_skb() gets called and the ethernet
> header never got written, resulting in the skb having a data section
> without a proper mac header. Current check:
>
> if (unlikely(skb->mac_header >= skb->network_header || skb->len == 0))
> {
> **drop packet**
> }
>
> in __bpf_redirect_common() is insufficient since it only checks if the
> mac header is misordered or if the data length is 0. So, any packet
> with a malformed MAC header that is not 14 bytes but is not 0 doesn't
> get dropped. Adding bounds checks for mac header size should fix this.
> And from what I see in the syz test of this patch, it does.
>
> Are there any possible unexpected issues that can be caused by this?