Fix a KMSAN kernel-infoleak detected  by the syzbot .

[net?] KMSAN: kernel-infoleak in __skb_datagram_iter

In tcf_ife_dump(), the variable 'opt' was partially initialized using a
designatied initializer. While the padding bytes are reamined
uninitialized. nla_put() copies the entire structure into a
netlink message, these uninitialized bytes leaked to userspace.

Initialize the structure with memset before assigning its fields
to ensure all members and padding are cleared prior to beign copied.

Signed-off-by: Ranganath V N <vnranganath.20@gmail.com>
---
Changes in v2:
- removed memset(&t, 0, sizeof(t)) from previous patch.
- added the new patch series to address the issue.
- Link to v1: https://lore.kernel.org/r/20251031-infoleak-v1-1-9f7250ee33aa@gmail.com

---
Ranganath V N (2):
      net: sched: act_ife: initialize struct tc_ife to fix KMSAN kernel-infoleak
      net: sched: act_connmark: zero initialize the struct to avoid KMSAN

 net/sched/act_connmark.c | 12 +++++++-----
 net/sched/act_ife.c      | 12 +++++++-----
 2 files changed, 14 insertions(+), 10 deletions(-)
---
base-commit: d127176862a93c4b3216bda533d2bee170af5e71
change-id: 20251031-infoleak-8a7de6afc987

Best regards,
-- 
Ranganath V N <vnranganath.20@gmail.com>

On Sat, Nov 01, 2025 at 06:04:46PM +0530, Ranganath V N wrote:
> Fix a KMSAN kernel-infoleak detected  by the syzbot .
> 
> [net?] KMSAN: kernel-infoleak in __skb_datagram_iter
> 
> In tcf_ife_dump(), the variable 'opt' was partially initialized using a
> designatied initializer. While the padding bytes are reamined
> uninitialized. nla_put() copies the entire structure into a
> netlink message, these uninitialized bytes leaked to userspace.
> 
> Initialize the structure with memset before assigning its fields
> to ensure all members and padding are cleared prior to beign copied.

Perhaps not important, but this seems to only describe patch 1/2.

> 
> Signed-off-by: Ranganath V N <vnranganath.20@gmail.com>

Sorry for not looking more carefully at v1.

The presence of this padding seems pretty subtle to me.
And while I agree that your change fixes the problem described.
I wonder if it would be better to make things more obvious
by adding a 2-byte pad member to the structures involved.

...

On 11/4/25 19:38, Simon Horman wrote:
> On Sat, Nov 01, 2025 at 06:04:46PM +0530, Ranganath V N wrote:
>> Fix a KMSAN kernel-infoleak detected  by the syzbot .
>>
>> [net?] KMSAN: kernel-infoleak in __skb_datagram_iter
>>
>> In tcf_ife_dump(), the variable 'opt' was partially initialized using a
>> designatied initializer. While the padding bytes are reamined
>> uninitialized. nla_put() copies the entire structure into a
>> netlink message, these uninitialized bytes leaked to userspace.
>>
>> Initialize the structure with memset before assigning its fields
>> to ensure all members and padding are cleared prior to beign copied.
>
> Perhaps not important, but this seems to only describe patch 1/2.
>
>>
>> Signed-off-by: Ranganath V N <vnranganath.20@gmail.com>
>
> Sorry for not looking more carefully at v1.
>
> The presence of this padding seems pretty subtle to me.
> And while I agree that your change fixes the problem described.
> I wonder if it would be better to make things more obvious
> by adding a 2-byte pad member to the structures involved.

Thanks for the input.

One question — even though adding a 2-byte `pad` field silences KMSAN,
would that approach be reliable across all architectures?
Since the actual amount and placement of padding can vary depending on
structure alignment and compiler behavior, I’m wondering if this would only
silence the report on certain builds rather than fixing the root cause.

The current memset-based initialization explicitly clears all bytes in the
structure (including any compiler-inserted padding), which seems safer and
more consistent across architectures.

Also, adding a new member — even a padding field — could potentially alter
the structure size or layout as seen from user space. That might 
unintentionally affect existing user-space expectations.

Do you think relying on a manual pad field is good enough?

regards,  
--Ranganath

On Wed, Nov 05, 2025 at 03:33:58PM +0530, Ranganath V N wrote:
> On 11/4/25 19:38, Simon Horman wrote:
> > On Sat, Nov 01, 2025 at 06:04:46PM +0530, Ranganath V N wrote:
> >> Fix a KMSAN kernel-infoleak detected  by the syzbot .
> >>
> >> [net?] KMSAN: kernel-infoleak in __skb_datagram_iter
> >>
> >> In tcf_ife_dump(), the variable 'opt' was partially initialized using a
> >> designatied initializer. While the padding bytes are reamined
> >> uninitialized. nla_put() copies the entire structure into a
> >> netlink message, these uninitialized bytes leaked to userspace.
> >>
> >> Initialize the structure with memset before assigning its fields
> >> to ensure all members and padding are cleared prior to beign copied.
> >
> > Perhaps not important, but this seems to only describe patch 1/2.
> >
> >>
> >> Signed-off-by: Ranganath V N <vnranganath.20@gmail.com>
> >
> > Sorry for not looking more carefully at v1.
> >
> > The presence of this padding seems pretty subtle to me.
> > And while I agree that your change fixes the problem described.
> > I wonder if it would be better to make things more obvious
> > by adding a 2-byte pad member to the structures involved.
> 
> Thanks for the input.
> 
> One question — even though adding a 2-byte `pad` field silences KMSAN,
> would that approach be reliable across all architectures?
> Since the actual amount and placement of padding can vary depending on
> structure alignment and compiler behavior, I’m wondering if this would only
> silence the report on certain builds rather than fixing the root cause.
> 
> The current memset-based initialization explicitly clears all bytes in the
> structure (including any compiler-inserted padding), which seems safer and
> more consistent across architectures.
> 
> Also, adding a new member — even a padding field — could potentially alter
> the structure size or layout as seen from user space. That might 
> unintentionally affect existing user-space expectations.
> 
> Do you think relying on a manual pad field is good enough?

I think these are the right questions to ask.

My thinking is that structures will be padded to a multiple
of either 4 or 8 bytes, depending on the architecture.

And my observation is that that the unpadded length of both of the structures
in question are 22 bytes. And that on x86_64 they are padded to 24 bytes.
Which is divisible by both 4 and 8. So I assume this will be consistent
for all architectures. If so, I think this would address the questions you
raised.

I do, however, agree that your current memset-based approach is safer
in the sense that it carries a lower risk of breaking things because
it has fewer assumptions (that we have thought of so far).

On Wed, Nov 5, 2025 at 7:59 AM Simon Horman <horms@kernel.org> wrote:
>
> On Wed, Nov 05, 2025 at 03:33:58PM +0530, Ranganath V N wrote:
> > On 11/4/25 19:38, Simon Horman wrote:
> > > On Sat, Nov 01, 2025 at 06:04:46PM +0530, Ranganath V N wrote:
> > >> Fix a KMSAN kernel-infoleak detected  by the syzbot .
> > >>
> > >> [net?] KMSAN: kernel-infoleak in __skb_datagram_iter
> > >>
> > >> In tcf_ife_dump(), the variable 'opt' was partially initialized using a
> > >> designatied initializer. While the padding bytes are reamined
> > >> uninitialized. nla_put() copies the entire structure into a
> > >> netlink message, these uninitialized bytes leaked to userspace.
> > >>
> > >> Initialize the structure with memset before assigning its fields
> > >> to ensure all members and padding are cleared prior to beign copied.
> > >
> > > Perhaps not important, but this seems to only describe patch 1/2.
> > >
> > >>
> > >> Signed-off-by: Ranganath V N <vnranganath.20@gmail.com>
> > >
> > > Sorry for not looking more carefully at v1.
> > >
> > > The presence of this padding seems pretty subtle to me.
> > > And while I agree that your change fixes the problem described.
> > > I wonder if it would be better to make things more obvious
> > > by adding a 2-byte pad member to the structures involved.
> >
> > Thanks for the input.
> >
> > One question — even though adding a 2-byte `pad` field silences KMSAN,
> > would that approach be reliable across all architectures?
> > Since the actual amount and placement of padding can vary depending on
> > structure alignment and compiler behavior, I’m wondering if this would only
> > silence the report on certain builds rather than fixing the root cause.
> >
> > The current memset-based initialization explicitly clears all bytes in the
> > structure (including any compiler-inserted padding), which seems safer and
> > more consistent across architectures.
> >
> > Also, adding a new member — even a padding field — could potentially alter
> > the structure size or layout as seen from user space. That might
> > unintentionally affect existing user-space expectations.
> >
> > Do you think relying on a manual pad field is good enough?
>
> I think these are the right questions to ask.
>
> My thinking is that structures will be padded to a multiple
> of either 4 or 8 bytes, depending on the architecture.
>
> And my observation is that that the unpadded length of both of the structures
> in question are 22 bytes. And that on x86_64 they are padded to 24 bytes.
> Which is divisible by both 4 and 8. So I assume this will be consistent
> for all architectures. If so, I think this would address the questions you
> raised.
>
> I do, however, agree that your current memset-based approach is safer
> in the sense that it carries a lower risk of breaking things because
> it has fewer assumptions (that we have thought of so far).

+1
My view is lets fix the immediate leak issue with the memset, and a
subsequent patch can add the padding if necessary.

cheers,
jamal

On Wed, Nov 05, 2025 at 10:09:37AM -0500, Jamal Hadi Salim wrote:
> On Wed, Nov 5, 2025 at 7:59 AM Simon Horman <horms@kernel.org> wrote:
> >
> > On Wed, Nov 05, 2025 at 03:33:58PM +0530, Ranganath V N wrote:
> > > On 11/4/25 19:38, Simon Horman wrote:
> > > > On Sat, Nov 01, 2025 at 06:04:46PM +0530, Ranganath V N wrote:
> > > >> Fix a KMSAN kernel-infoleak detected  by the syzbot .
> > > >>
> > > >> [net?] KMSAN: kernel-infoleak in __skb_datagram_iter
> > > >>
> > > >> In tcf_ife_dump(), the variable 'opt' was partially initialized using a
> > > >> designatied initializer. While the padding bytes are reamined
> > > >> uninitialized. nla_put() copies the entire structure into a
> > > >> netlink message, these uninitialized bytes leaked to userspace.
> > > >>
> > > >> Initialize the structure with memset before assigning its fields
> > > >> to ensure all members and padding are cleared prior to beign copied.
> > > >
> > > > Perhaps not important, but this seems to only describe patch 1/2.
> > > >
> > > >>
> > > >> Signed-off-by: Ranganath V N <vnranganath.20@gmail.com>
> > > >
> > > > Sorry for not looking more carefully at v1.
> > > >
> > > > The presence of this padding seems pretty subtle to me.
> > > > And while I agree that your change fixes the problem described.
> > > > I wonder if it would be better to make things more obvious
> > > > by adding a 2-byte pad member to the structures involved.
> > >
> > > Thanks for the input.
> > >
> > > One question — even though adding a 2-byte `pad` field silences KMSAN,
> > > would that approach be reliable across all architectures?
> > > Since the actual amount and placement of padding can vary depending on
> > > structure alignment and compiler behavior, I’m wondering if this would only
> > > silence the report on certain builds rather than fixing the root cause.
> > >
> > > The current memset-based initialization explicitly clears all bytes in the
> > > structure (including any compiler-inserted padding), which seems safer and
> > > more consistent across architectures.
> > >
> > > Also, adding a new member — even a padding field — could potentially alter
> > > the structure size or layout as seen from user space. That might
> > > unintentionally affect existing user-space expectations.
> > >
> > > Do you think relying on a manual pad field is good enough?
> >
> > I think these are the right questions to ask.
> >
> > My thinking is that structures will be padded to a multiple
> > of either 4 or 8 bytes, depending on the architecture.
> >
> > And my observation is that that the unpadded length of both of the structures
> > in question are 22 bytes. And that on x86_64 they are padded to 24 bytes.
> > Which is divisible by both 4 and 8. So I assume this will be consistent
> > for all architectures. If so, I think this would address the questions you
> > raised.
> >
> > I do, however, agree that your current memset-based approach is safer
> > in the sense that it carries a lower risk of breaking things because
> > it has fewer assumptions (that we have thought of so far).
> 
> +1
> My view is lets fix the immediate leak issue with the memset, and a
> subsequent patch can add the padding if necessary.

Sure, no objections from my side.

On 11/6/25 00:43, Simon Horman wrote:
> On Wed, Nov 05, 2025 at 10:09:37AM -0500, Jamal Hadi Salim wrote:
>> On Wed, Nov 5, 2025 at 7:59 AM Simon Horman <horms@kernel.org> wrote:
>>>
>>> On Wed, Nov 05, 2025 at 03:33:58PM +0530, Ranganath V N wrote:
>>>> On 11/4/25 19:38, Simon Horman wrote:
>>>>> On Sat, Nov 01, 2025 at 06:04:46PM +0530, Ranganath V N wrote:
>>>>>> Fix a KMSAN kernel-infoleak detected  by the syzbot .
>>>>>>
>>>>>> [net?] KMSAN: kernel-infoleak in __skb_datagram_iter
>>>>>>
>>>>>> In tcf_ife_dump(), the variable 'opt' was partially initialized using a
>>>>>> designatied initializer. While the padding bytes are reamined
>>>>>> uninitialized. nla_put() copies the entire structure into a
>>>>>> netlink message, these uninitialized bytes leaked to userspace.
>>>>>>
>>>>>> Initialize the structure with memset before assigning its fields
>>>>>> to ensure all members and padding are cleared prior to beign copied.
>>>>>
>>>>> Perhaps not important, but this seems to only describe patch 1/2.
>>>>>
>>>>>>
>>>>>> Signed-off-by: Ranganath V N <vnranganath.20@gmail.com>
>>>>>
>>>>> Sorry for not looking more carefully at v1.
>>>>>
>>>>> The presence of this padding seems pretty subtle to me.
>>>>> And while I agree that your change fixes the problem described.
>>>>> I wonder if it would be better to make things more obvious
>>>>> by adding a 2-byte pad member to the structures involved.
>>>>
>>>> Thanks for the input.
>>>>
>>>> One question — even though adding a 2-byte `pad` field silences KMSAN,
>>>> would that approach be reliable across all architectures?
>>>> Since the actual amount and placement of padding can vary depending on
>>>> structure alignment and compiler behavior, I’m wondering if this would only
>>>> silence the report on certain builds rather than fixing the root cause.
>>>>
>>>> The current memset-based initialization explicitly clears all bytes in the
>>>> structure (including any compiler-inserted padding), which seems safer and
>>>> more consistent across architectures.
>>>>
>>>> Also, adding a new member — even a padding field — could potentially alter
>>>> the structure size or layout as seen from user space. That might
>>>> unintentionally affect existing user-space expectations.
>>>>
>>>> Do you think relying on a manual pad field is good enough?
>>>
>>> I think these are the right questions to ask.
>>>
>>> My thinking is that structures will be padded to a multiple
>>> of either 4 or 8 bytes, depending on the architecture.
>>>
>>> And my observation is that that the unpadded length of both of the structures
>>> in question are 22 bytes. And that on x86_64 they are padded to 24 bytes.
>>> Which is divisible by both 4 and 8. So I assume this will be consistent
>>> for all architectures. If so, I think this would address the questions you
>>> raised.
>>>
>>> I do, however, agree that your current memset-based approach is safer
>>> in the sense that it carries a lower risk of breaking things because
>>> it has fewer assumptions (that we have thought of so far).
>>
>> +1
>> My view is lets fix the immediate leak issue with the memset, and a
>> subsequent patch can add the padding if necessary.
>
> Sure, no objections from my side.

Thanks for the clarification.

I'll send the new patch series(v3) with fix(missed ;)
I'll keep the current change limited to the memset fix to resolve the
issue. Also, I've noticed that similar uninitialized structure
patterns exist in a few other locations in the net code.

Thanks for the review.