[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89iJL3upMfHB+DsuS8Ux06fM36dbHeaU5xof5-T+Fe80tFg@mail.gmail.com>
Date: Fri, 31 Oct 2025 06:04:45 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Ranganath V N <vnranganath.20@...il.com>
Cc: Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>, 
	Jiri Pirko <jiri@...nulli.us>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, skhan@...uxfoundation.org, 
	david.hunter.linux@...il.com, khalid@...nel.org, 
	syzbot+0c85cae3350b7d486aee@...kaller.appspotmail.com
Subject: Re: [PATCH] net: sched: act_ife: initialize struct tc_ife to fix
 KMSAN kernel-infoleak
On Fri, Oct 31, 2025 at 5:24 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()
> +
> +       opt.index = ife->tcf_index,
> +       opt.refcnt = refcount_read(&ife->tcf_refcnt) - ref,
> +       opt.bindcnt = atomic_read(&ife->tcf_bindcnt) - bind,
> +
>         spin_lock_bh(&ife->tcf_lock);
>         opt.action = ife->tcf_action;
>         p = rcu_dereference_protected(ife->params,
>
> ---
> base-commit: d127176862a93c4b3216bda533d2bee170af5e71
> change-id: 20251031-infoleak-8a7de6afc987
>
> Best regards,
> --
> Ranganath V N <vnranganath.20@...il.com>
>
Powered by blists - more mailing lists
 
