netdevsim: Initialize all fields of ip header when building dummy sk_buff

[PATCH net] netdevsim: Initialize all fields of ip header when building dummy sk_buff

Posted by Nikola Z. Ivanov 1 month, 4 weeks ago

Syzbot reports a KMSAN uninit-value originating from
nsim_dev_trap_skb_build, with the allocation also
being performed in the same function.

The cause of the KMSAN warning is a missing assignment of
the tos and id fields of the ip header.

Fix this by calling skb_put_zero instead of skb_put to
guarantee null initialization.
Additionally remove the now redundant zero assignments
and reorder the remaining ones so that they more closely
match the order of the fields as they appear in the ip header.

Closes: https://syzkaller.appspot.com/bug?extid=23d7fcd204e3837866ff
Fixes: da58f90f11f5 ("netdevsim: Add devlink-trap support")
Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
---
There is a very similar function in psample.c called nsim_dev_psample_skb_build
which is almost identical to nsim_dev_trap_skb_build except for the
allocation flag reflecting its non-interrupt context and the fact
it does proper initialization of all fields.
Since these 2 are almost identical would it make sense to combine them
into 1, possbly by passing the allocation flags as parameters?

Thank you in advance for reviewing and answering!

 drivers/net/netdevsim/dev.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 1e06e781c835..64b7cc3a6575 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -829,16 +829,14 @@ static struct sk_buff *nsim_dev_trap_skb_build(void)
 	skb->protocol = htons(ETH_P_IP);
 
 	skb_set_network_header(skb, skb->len);
-	iph = skb_put(skb, sizeof(struct iphdr));
-	iph->protocol = IPPROTO_UDP;
-	iph->saddr = in_aton("192.0.2.1");
-	iph->daddr = in_aton("198.51.100.1");
-	iph->version = 0x4;
-	iph->frag_off = 0;
+	iph = skb_put_zero(skb, sizeof(struct iphdr));
 	iph->ihl = 0x5;
+	iph->version = 0x4;
 	iph->tot_len = htons(tot_len);
 	iph->ttl = 100;
-	iph->check = 0;
+	iph->protocol = IPPROTO_UDP;
+	iph->saddr = in_aton("192.0.2.1");
+	iph->daddr = in_aton("198.51.100.1");
 	iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
 
 	skb_set_transport_header(skb, skb->len);
-- 
2.53.0

Re: [PATCH net] netdevsim: Initialize all fields of ip header when building dummy sk_buff

Posted by Jakub Kicinski 1 month, 4 weeks ago

On Tue, 21 Apr 2026 10:37:38 +0300 Nikola Z. Ivanov wrote:
> Additionally remove the now redundant zero assignments
> and reorder the remaining ones so that they more closely
> match the order of the fields as they appear in the ip header.

Doesn't matter, now that the whole thing is zero-initialized.
I don't think it's worth the noise in the git history.
-- 
pw-bot: cr

Re: [PATCH net] netdevsim: Initialize all fields of ip header when building dummy sk_buff

Posted by Jakub Kicinski 1 month, 3 weeks ago

On Tue, 21 Apr 2026 08:20:05 -0700 Jakub Kicinski wrote:
> On Tue, 21 Apr 2026 10:37:38 +0300 Nikola Z. Ivanov wrote:
> > Additionally remove the now redundant zero assignments
> > and reorder the remaining ones so that they more closely
> > match the order of the fields as they appear in the ip header.  
> 
> Doesn't matter, now that the whole thing is zero-initialized.
> I don't think it's worth the noise in the git history.

I don't see a v2, so just to be clear -- the patch looks good,
just don't reorder the assignments please.

Re: [PATCH net] netdevsim: Initialize all fields of ip header when building dummy sk_buff

Posted by Breno Leitao 1 month, 4 weeks ago

On Tue, Apr 21, 2026 at 10:37:38AM +0300, Nikola Z. Ivanov wrote:
> Syzbot reports a KMSAN uninit-value originating from
> nsim_dev_trap_skb_build, with the allocation also
> being performed in the same function.
> 
> The cause of the KMSAN warning is a missing assignment of
> the tos and id fields of the ip header.
> 
> Fix this by calling skb_put_zero instead of skb_put to
> guarantee null initialization.

> Additionally remove the now redundant zero assignments
> and reorder the remaining ones so that they more closely
> match the order of the fields as they appear in the ip header.
> 
> Closes: https://syzkaller.appspot.com/bug?extid=23d7fcd204e3837866ff

How do you check in the report above that the missig un-initialized
fields are "tos" and "id"?

Thanks for the fix,
--breno

Re: [PATCH net] netdevsim: Initialize all fields of ip header when building dummy sk_buff

Posted by Nikola Z. Ivanov 1 month, 4 weeks ago


On 4/21/26 11:19 AM, Breno Leitao wrote:
> On Tue, Apr 21, 2026 at 10:37:38AM +0300, Nikola Z. Ivanov wrote:
>> Syzbot reports a KMSAN uninit-value originating from
>> nsim_dev_trap_skb_build, with the allocation also
>> being performed in the same function.
>>
>> The cause of the KMSAN warning is a missing assignment of
>> the tos and id fields of the ip header.
>>
>> Fix this by calling skb_put_zero instead of skb_put to
>> guarantee null initialization.
>> Additionally remove the now redundant zero assignments
>> and reorder the remaining ones so that they more closely
>> match the order of the fields as they appear in the ip header.
>>
>> Closes: https://syzkaller.appspot.com/bug?extid=23d7fcd204e3837866ff
> How do you check in the report above that the missig un-initialized
> fields are "tos" and "id"?
>
> Thanks for the fix,
> --breno
Hi Breno,

I don't think it is visible here, my guess would
be because the checksum calculator walks the
header in small chunks instead of referencing
its fields.

The whole "KMSAN: uninit-value in irqentry_exit_to_kernel_mode_preempt"
doesn't really sound quite right.

Thank you!

Re: [PATCH net] netdevsim: Initialize all fields of ip header when building dummy sk_buff

Posted by Breno Leitao 1 month, 4 weeks ago

On Tue, Apr 21, 2026 at 11:54:19AM +0300, Nikola Z. Ivanov wrote:
> On 4/21/26 11:19 AM, Breno Leitao wrote:
> > On Tue, Apr 21, 2026 at 10:37:38AM +0300, Nikola Z. Ivanov wrote:
> > >
> > > Closes: https://syzkaller.appspot.com/bug?extid=23d7fcd204e3837866ff
> >
> > How do you check in the report above that the missig un-initialized
> > fields are "tos" and "id"?
> 
> I don't think it is visible here, my guess would
> be because the checksum calculator walks the
> header in small chunks instead of referencing
> its fields.
> 
> The whole "KMSAN: uninit-value in irqentry_exit_to_kernel_mode_preempt"
> doesn't really sound quite right.

That's precisely my question - how does this fix relate to that specific
report?

Were you able to reproduce the KMSAN report?

Thanks for the quick answer,
--breno

Re: [PATCH net] netdevsim: Initialize all fields of ip header when building dummy sk_buff

Posted by Nikola Z. Ivanov 1 month, 4 weeks ago


On 4/21/26 12:12 PM, Breno Leitao wrote:
> On Tue, Apr 21, 2026 at 11:54:19AM +0300, Nikola Z. Ivanov wrote:
>> On 4/21/26 11:19 AM, Breno Leitao wrote:
>>> On Tue, Apr 21, 2026 at 10:37:38AM +0300, Nikola Z. Ivanov wrote:
>>>> Closes: https://syzkaller.appspot.com/bug?extid=23d7fcd204e3837866ff
>>> How do you check in the report above that the missig un-initialized
>>> fields are "tos" and "id"?
>> I don't think it is visible here, my guess would
>> be because the checksum calculator walks the
>> header in small chunks instead of referencing
>> its fields.
>>
>> The whole "KMSAN: uninit-value in irqentry_exit_to_kernel_mode_preempt"
>> doesn't really sound quite right.
> That's precisely my question - how does this fix relate to that specific
> report?
The fact that the 2 call traces from the allocation and usage
have a common origin in nsim_dev_trap_skb_build sort of gives it away.
Just to be clear, I saw the syzbot report and started investigating from
there, not the other way around.
> Were you able to reproduce the KMSAN report?
>
> Thanks for the quick answer,
> --breno
Yes, but it is a bit inconsistent.

Just booting the disk from the report and adding a device
is enough to trigger it, but we have to wait for some time:

syzkaller
syzkaller login: root
# echo "1 1" > /sys/bus/netdevsim/new_device
# [  726.477183][ T5462] 8021q: adding VLAN 0 to HW filter on device eth1

# [ 1845.100611][   T80] 
=====================================================
[ 1845.102363][   T80] BUG: KMSAN: uninit-value in 
irqentry_exit_to_kernel_mode_preempt+0x8f/0xa0
[ 1845.104209][   T80] irqentry_exit_to_kernel_mode_preempt+0x8f/0xa0
[ 1845.105594][   T80]  irqentry_exit+0x7c/0x7b0
[ 1845.106629][   T80]  sysvec_apic_timer_interrupt+0x52/0x90
[ 1845.107829][   T80]  asm_sysvec_apic_timer_interrupt+0x1f/0x30
[ 1845.108959][   T80]  srso_alias_safe_ret+0x0/0x7
[ 1845.108959][   T80]  __msan_metadata_ptr_for_load_4+0x24/0x40
[ 1845.108959][   T80]  ip_fast_csum+0x1e6/0x3f0
[ 1845.108959][   T80]  nsim_dev_trap_report_work+0x8c0/0x1430
[ 1845.108959][   T80]  process_scheduled_works+0xbdb/0x1e20
[ 1845.108959][   T80]  worker_thread+0xee5/0x1590
[ 1845.108959][   T80]  kthread+0x540/0x600
[ 1845.108959][   T80]  ret_from_fork+0x210/0x8f0
[ 1845.108959][   T80]  ret_from_fork_asm+0x1a/0x30
[ 1845.108959][   T80]
[ 1845.108959][   T80] Uninit was created at:
[ 1845.108959][   T80] __kmalloc_node_track_caller_noprof+0x4fb/0x1770
[ 1845.108959][   T80]  __alloc_skb+0x90d/0x1190
[ 1845.108959][   T80]  nsim_dev_trap_report_work+0x3f2/0x1430
[ 1845.108959][   T80]  process_scheduled_works+0xbdb/0x1e20
[ 1845.108959][   T80]  worker_thread+0xee5/0x1590
[ 1845.108959][   T80]  kthread+0x540/0x600
[ 1845.108959][   T80]  ret_from_fork+0x210/0x8f0
[ 1845.108959][   T80]  ret_from_fork_asm+0x1a/0x30
[ 1845.108959][   T80]


Thank you,
Nikola