linux-kernel - Re: [PATCH] net: sched: act_ife: initialize struct tc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89iKrhc8VYJRW+bRZ8AzaXsdq4Gem8PgFR9d7b-CVrjDr7g@mail.gmail.com>
Date: Fri, 31 Oct 2025 09:00:33 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Ranganath V N <vnranganath.20@...il.com>
Cc: davem@...emloft.net, david.hunter.linux@...il.com, horms@...nel.org, 
	jhs@...atatu.com, jiri@...nulli.us, khalid@...nel.org, kuba@...nel.org, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org, pabeni@...hat.com, 
	skhan@...uxfoundation.org, 
	syzbot+0c85cae3350b7d486aee@...kaller.appspotmail.com, 
	xiyou.wangcong@...il.com
Subject: Re: [PATCH] net: sched: act_ife: initialize struct tc_ife to fix
 KMSAN kernel-infoleak

On Fri, Oct 31, 2025 at 8:56 AM Ranganath V N <vnranganath.20@...il.com> 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.
> >>
> >> This change silences the KMSAN report and prevents potential information
> >> leaks from the kernel memory.
> >>
> >> This fix has been tested and validated by syzbot. This patch closes the
> >> bug reported at the following syzkaller link and ensures no infoleak.
> >>
> >> Reported-by: syzbot+0c85cae3350b7d486aee@...kaller.appspotmail.com
> >> Closes: https://syzkaller.appspot.com/bug?extid=0c85cae3350b7d486aee
> >> Tested-by: syzbot+0c85cae3350b7d486aee@...kaller.appspotmail.com
> >> Fixes: ef6980b6becb ("introduce IFE action")
> >> Signed-off-by: Ranganath V N <vnranganath.20@...il.com>
> >> ---
> >> 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.
> >> ---
> >>  net/sched/act_ife.c | 13 ++++++++-----
> >>  1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> >> index 107c6d83dc5c..608ef6cc2224 100644
> >> --- a/net/sched/act_ife.c
> >> +++ b/net/sched/act_ife.c
> >> @@ -644,13 +644,16 @@ static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind,
> >>         unsigned char *b = skb_tail_pointer(skb);
> >>         struct tcf_ife_info *ife = to_ife(a);
> >>         struct tcf_ife_params *p;
> >> -       struct tc_ife opt = {
> >> -               .index = ife->tcf_index,
> >> -               .refcnt = refcount_read(&ife->tcf_refcnt) - ref,
> >> -               .bindcnt = atomic_read(&ife->tcf_bindcnt) - bind,
> >> -       };
> >> +       struct tc_ife opt;
> >>         struct tcf_t t;
> >>
> >> +       memset(&opt, 0, sizeof(opt));
> >> +       memset(&t, 0, sizeof(t));
> >
> >Why is the second memset() needed ? Please do not add unrelated changes.
> >
> >Also I would also fix tcf_connmark_dump()
>
> Hi Eric,
> Do you want me fix tcf_connmark_dump() in this patch or new?

Send a series of two patches, thank you.