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?
