* [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