Re: [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS

[Eric Dumazet]

On Wed, Jun 3, 2026 at 1:16 AM Zhou, Yun <yun.zhou@xxxxxxxxxxxxx> wrote:
>
>
>
> On 6/3/26 13:44, Eric Dumazet wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> > On Tue, Jun 2, 2026 at 8:08 PM Yun Zhou <yun.zhou@xxxxxxxxxxxxx> wrote:
> > Please add a Closes: tag
> I will add a Closes link in v2.
> Closes: https://syzkaller.appspot.com/bug?extid=fa2f5b1fb06147be5e16
> >
> > I found some not relevant syzbot report :
> >
> > https://lore.kernel.org/netdev/6a196faf.c16d89a8.217f2c.0002.GAE@xxxxxxxxxx/
> This should be the same issue. And it can be reproduced by
> https://syzkaller.appspot.com/text?tag=ReproC&x=12924152580000

Please investigate which device allowed to send an Ethernet packet
smaller than the ethernet header.

We do not want to add tests all over the places, we should fix the origin.

Look for dev->min_header_len

Thanks.

> >
> >> Fixes: 67a900cc0436 ("flow_dissector: introduce support for Ethernet addresses")
> >> Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>
> >> ---
> >>   net/core/flow_dissector.c | 10 ++++++++--
> >>   1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> >> index 2a98f5fa74eb..d5817b800079 100644
> >> --- a/net/core/flow_dissector.c
> >> +++ b/net/core/flow_dissector.c
> >> @@ -1173,13 +1173,19 @@ bool __skb_flow_dissect(const struct net *net,
> >>
> >>          if (dissector_uses_key(flow_dissector,
> >>                                 FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
> >> -               struct ethhdr *eth = eth_hdr(skb);
> >>                  struct flow_dissector_key_eth_addrs *key_eth_addrs;
> >>
> >>                  key_eth_addrs = skb_flow_dissector_target(flow_dissector,
> >>                                                            FLOW_DISSECTOR_KEY_ETH_ADDRS,
> >>                                                            target_container);
> >> -               memcpy(key_eth_addrs, eth, sizeof(*key_eth_addrs));
> >> +               /* Ensure the skb has enough data at mac_header to cover
> >> +                * both src and dst Ethernet addresses.
> >> +                */
> >> +               if (skb_mac_header_was_set(skb) &&
> >> +                   skb_tail_pointer(skb) - skb_mac_header(skb) >= sizeof(*key_eth_addrs))
> >> +                       memcpy(key_eth_addrs, eth_hdr(skb), sizeof(*key_eth_addrs));
> >> +               else
> >> +                       memset(key_eth_addrs, 0, sizeof(*key_eth_addrs));
> >>          }
> > Can you show us a stack trace, why mac_header would not be set at this point?
> It seems that there is no need to call skb_mac_header_was_set(skb).