| Message ID | 20241104040218.193632-1-danielyangkang@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [net] Drop packets with invalid headers to prevent KMSAN infoleak | expand |
On Mon, Nov 4, 2024 at 5:02 AM Daniel Yang <danielyangkang@gmail.com> wrote: > > KMSAN detects uninitialized memory stored to memory by > bpf_clone_redirect(). Adding a check to the transmission path to find > malformed headers prevents this issue. Specifically, we check if the length > of the data stored in skb is less than the minimum device header length. If > so, drop the packet since the skb cannot contain a valid device header. > Also check if mac_header_len(skb) is outside the range provided of valid > device header lengths. > > Testing this patch with syzbot removes the bug. > > Macro added to not affect normal builds. > > Fixes: 88264981f208 ("Merge tag 'sched_ext-for-6.12' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext") > Reported-by: syzbot+346474e3bf0b26bd3090@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=346474e3bf0b26bd3090 > Signed-off-by: Daniel Yang <danielyangkang@gmail.com> > --- > v1: Enclosed in macro to not affect normal builds > > net/core/filter.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/net/core/filter.c b/net/core/filter.c > index cd3524cb3..9c5786f9c 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -2191,6 +2191,14 @@ static int __bpf_redirect_common(struct sk_buff *skb, struct net_device *dev, > return -ERANGE; > } > > +#if IS_ENABLED(CONFIG_KMSAN) > + if (unlikely(skb->len < dev->min_header_len || > + skb_mac_header_len(skb) < dev->min_header_len || > + skb_mac_header_len(skb) > dev->hard_header_len)) { > + kfree_skb(skb); > + return -ERANGE; > + } > +#endif > bpf_push_mac_rcsum(skb); > return flags & BPF_F_INGRESS ? > __bpf_rx_skb(dev, skb) : __bpf_tx_skb(dev, skb); > -- > 2.39.2 > I am not a BPF maintainer, but for the record I think it is wrong to silence KMSAN and give the impression a bug is 'removed'.
diff --git a/net/core/filter.c b/net/core/filter.c index cd3524cb3..9c5786f9c 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2191,6 +2191,14 @@ static int __bpf_redirect_common(struct sk_buff *skb, struct net_device *dev, return -ERANGE; } +#if IS_ENABLED(CONFIG_KMSAN) + if (unlikely(skb->len < dev->min_header_len || + skb_mac_header_len(skb) < dev->min_header_len || + skb_mac_header_len(skb) > dev->hard_header_len)) { + kfree_skb(skb); + return -ERANGE; + } +#endif bpf_push_mac_rcsum(skb); return flags & BPF_F_INGRESS ? __bpf_rx_skb(dev, skb) : __bpf_tx_skb(dev, skb);
KMSAN detects uninitialized memory stored to memory by bpf_clone_redirect(). Adding a check to the transmission path to find malformed headers prevents this issue. Specifically, we check if the length of the data stored in skb is less than the minimum device header length. If so, drop the packet since the skb cannot contain a valid device header. Also check if mac_header_len(skb) is outside the range provided of valid device header lengths. Testing this patch with syzbot removes the bug. Macro added to not affect normal builds. Fixes: 88264981f208 ("Merge tag 'sched_ext-for-6.12' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext") Reported-by: syzbot+346474e3bf0b26bd3090@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=346474e3bf0b26bd3090 Signed-off-by: Daniel Yang <danielyangkang@gmail.com> --- v1: Enclosed in macro to not affect normal builds net/core/filter.c | 8 ++++++++ 1 file changed, 8 insertions(+)