Netdev List
 help / color / mirror / Atom feed
* [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS
@ 2026-06-03  3:08 Yun Zhou
  2026-06-03  5:44 ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Yun Zhou @ 2026-06-03  3:08 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, horms; +Cc: netdev, linux-kernel, yun.zhou

When dissecting FLOW_DISSECTOR_KEY_ETH_ADDRS, the code unconditionally
copies sizeof(flow_dissector_key_eth_addrs) (12 bytes) from eth_hdr(skb).
However, if the packet is too short (e.g., a 1-byte packet sent via
AF_PACKET on a TUN device), eth_hdr(skb) points to uninitialized skb
memory beyond the actual packet data.

This causes KMSAN to report uninit-value in __fl_lookup() when the
uninitialized eth addresses are used as rhashtable lookup key in
cls_flower.

Fix by checking that sufficient data exists from mac_header to skb tail
before copying. If not enough data, zero the key to ensure deterministic
behavior (no false matches).

Reported-by: syzbot+fa2f5b1fb06147be5e16@syzkaller.appspotmail.com
Fixes: 67a900cc0436 ("flow_dissector: introduce support for Ethernet addresses")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 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));
 	}
 
 	if (dissector_uses_key(flow_dissector,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS
  2026-06-03  3:08 [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS Yun Zhou
@ 2026-06-03  5:44 ` Eric Dumazet
  2026-06-03  8:15   ` Zhou, Yun
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-06-03  5:44 UTC (permalink / raw)
  To: Yun Zhou; +Cc: davem, kuba, pabeni, horms, netdev, linux-kernel

On Tue, Jun 2, 2026 at 8:08 PM Yun Zhou <yun.zhou@windriver.com> wrote:
>
> When dissecting FLOW_DISSECTOR_KEY_ETH_ADDRS, the code unconditionally
> copies sizeof(flow_dissector_key_eth_addrs) (12 bytes) from eth_hdr(skb).
> However, if the packet is too short (e.g., a 1-byte packet sent via
> AF_PACKET on a TUN device), eth_hdr(skb) points to uninitialized skb
> memory beyond the actual packet data.
>
> This causes KMSAN to report uninit-value in __fl_lookup() when the
> uninitialized eth addresses are used as rhashtable lookup key in
> cls_flower.
>
> Fix by checking that sufficient data exists from mac_header to skb tail
> before copying. If not enough data, zero the key to ensure deterministic
> behavior (no false matches).
>
> Reported-by: syzbot+fa2f5b1fb06147be5e16@syzkaller.appspotmail.com

Please add a Closes: tag

I found some not relevant syzbot report :

https://lore.kernel.org/netdev/6a196faf.c16d89a8.217f2c.0002.GAE@google.com/

> Fixes: 67a900cc0436 ("flow_dissector: introduce support for Ethernet addresses")
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> ---
>  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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS
  2026-06-03  5:44 ` Eric Dumazet
@ 2026-06-03  8:15   ` Zhou, Yun
  2026-06-03  8:33     ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Zhou, Yun @ 2026-06-03  8:15 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, kuba, pabeni, horms, netdev, linux-kernel



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@windriver.com> 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@google.com/
This should be the same issue. And it can be reproduced by 
https://syzkaller.appspot.com/text?tag=ReproC&x=12924152580000
>
>> Fixes: 67a900cc0436 ("flow_dissector: introduce support for Ethernet addresses")
>> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
>> ---
>>   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).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS
  2026-06-03  8:15   ` Zhou, Yun
@ 2026-06-03  8:33     ` Eric Dumazet
  2026-06-03  8:54       ` Jiayuan Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-06-03  8:33 UTC (permalink / raw)
  To: Zhou, Yun; +Cc: davem, kuba, pabeni, horms, netdev, linux-kernel

On Wed, Jun 3, 2026 at 1:16 AM Zhou, Yun <yun.zhou@windriver.com> 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@windriver.com> 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@google.com/
> 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@windriver.com>
> >> ---
> >>   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).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS
  2026-06-03  8:33     ` Eric Dumazet
@ 2026-06-03  8:54       ` Jiayuan Chen
  2026-06-03  9:22         ` Zhou, Yun
  0 siblings, 1 reply; 6+ messages in thread
From: Jiayuan Chen @ 2026-06-03  8:54 UTC (permalink / raw)
  To: Eric Dumazet, Zhou, Yun; +Cc: davem, kuba, pabeni, horms, netdev, linux-kernel


On 6/3/26 4:33 PM, Eric Dumazet wrote:
> On Wed, Jun 3, 2026 at 1:16 AM Zhou, Yun <yun.zhou@windriver.com> 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@windriver.com> 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@google.com/
>> 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.


It's TUN.

I think we should reject loading "tc filter ... flower eth_dst 
aa:bb:cc:dd:ee:ff ... " if dev->hard_header_len < 
sizeof(flow_dissector_key_eth_addrs)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS
  2026-06-03  8:54       ` Jiayuan Chen
@ 2026-06-03  9:22         ` Zhou, Yun
  0 siblings, 0 replies; 6+ messages in thread
From: Zhou, Yun @ 2026-06-03  9:22 UTC (permalink / raw)
  To: Jiayuan Chen, Eric Dumazet
  Cc: davem, kuba, pabeni, horms, netdev, linux-kernel



On 6/3/26 16:54, Jiayuan Chen 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 6/3/26 4:33 PM, Eric Dumazet wrote:
>> On Wed, Jun 3, 2026 at 1:16 AM Zhou, Yun <yun.zhou@windriver.com> 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@windriver.com> 
>>>> 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@google.com/ 
>>>>
>>> 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.
>
>
> It's TUN.
>
> I think we should reject loading "tc filter ... flower eth_dst
> aa:bb:cc:dd:ee:ff ... " if dev->hard_header_len <
> sizeof(flow_dissector_key_eth_addrs)
>
I will add checks in fl_change()

Thanks very much for your suggestion.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-03  9:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-03  3:08 [PATCH] flow_dissector: fix uninit-value in __skb_flow_dissect() for ETH_ADDRS Yun Zhou
2026-06-03  5:44 ` Eric Dumazet
2026-06-03  8:15   ` Zhou, Yun
2026-06-03  8:33     ` Eric Dumazet
2026-06-03  8:54       ` Jiayuan Chen
2026-06-03  9:22         ` Zhou, Yun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox