From: Simon Horman <horms@kernel.org>
To: Ranganath V N <vnranganath.20@gmail.com>
Cc: davem@davemloft.net, david.hunter.linux@gmail.com,
edumazet@google.com, jhs@mojatatu.com, jiri@resnulli.us,
khalid@kernel.org, kuba@kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, pabeni@redhat.com,
skhan@linuxfoundation.org,
syzbot+0c85cae3350b7d486aee@syzkaller.appspotmail.com,
xiyou.wangcong@gmail.com
Subject: Re: [PATCH v2 0/2] net: sched: act_ife: initialize struct tc_ife to fix KMSAN kernel-infoleak
Date: Wed, 5 Nov 2025 12:59:02 +0000 [thread overview]
Message-ID: <aQtKFtETfGBOPpCV@horms.kernel.org> (raw)
In-Reply-To: <20251105100403.17786-1-vnranganath.20@gmail.com>
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).
next prev parent reply other threads:[~2025-11-05 12:59 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-01 12:34 [PATCH v2 0/2] net: sched: act_ife: initialize struct tc_ife to fix KMSAN kernel-infoleak Ranganath V N
2025-11-01 12:34 ` [PATCH v2 1/2] " Ranganath V N
2025-11-04 14:10 ` Simon Horman
2025-11-01 12:34 ` [PATCH v2 2/2] net: sched: act_connmark: zero initialize the struct to avoid KMSAN Ranganath V N
2025-11-04 14:08 ` [PATCH v2 0/2] net: sched: act_ife: initialize struct tc_ife to fix KMSAN kernel-infoleak Simon Horman
2025-11-05 10:03 ` Ranganath V N
2025-11-05 12:59 ` Simon Horman [this message]
2025-11-05 15:09 ` Jamal Hadi Salim
2025-11-05 19:13 ` Simon Horman
2025-11-06 13:31 ` Ranganath V N
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aQtKFtETfGBOPpCV@horms.kernel.org \
--to=horms@kernel.org \
--cc=davem@davemloft.net \
--cc=david.hunter.linux@gmail.com \
--cc=edumazet@google.com \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=khalid@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=skhan@linuxfoundation.org \
--cc=syzbot+0c85cae3350b7d486aee@syzkaller.appspotmail.com \
--cc=vnranganath.20@gmail.com \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.