linux-kernel - Re: [PATCH v3 1/2] net: sched: act_connmark: 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: <aQ3MJ3ZPTxJqYPXT@horms.kernel.org>
Date: Fri, 7 Nov 2025 10:38:31 +0000
From: Simon Horman <horms@...nel.org>
To: Ranganath V N <vnranganath.20@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, jhs@...atatu.com,
	jiri@...nulli.us, kuba@...nel.org, pabeni@...hat.com,
	xiyou.wangcong@...il.com, david.hunter.linux@...il.com,
	khalid@...nel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, skhan@...uxfoundation.org,
	syzbot+0c85cae3350b7d486aee@...kaller.appspotmail.com
Subject: Re: [PATCH v3 1/2] net: sched: act_connmark: initialize struct
 tc_ife to fix kernel leak

On Fri, Nov 07, 2025 at 01:26:33AM +0530, Ranganath V N wrote:
> In tcf_connmark_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@...il.com>

Hi Ranganath,

Sorry for not noticing in my review of v2, but as this series fixes bugs in
code present in net it should be targeted at net.  This is done by
including net in the subject of each email, like this:

Subject: [PATCh net v3 1/2] ...

And this patch should have a fixes tag (patch 2/2 already has one).

Fixes: 22a5dc0e5e3e ("net: sched: Introduce connmark action")

Also, when posting v4, please be sure to wait until 24h have
elapsed since the posting of v3.

For more information about the above please see
https://docs.kernel.org/process/maintainer-netdev.html

> ---
>  net/sched/act_connmark.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
> index 3e89927d7116..2aaaaee9b6bb 100644
> --- a/net/sched/act_connmark.c
> +++ b/net/sched/act_connmark.c
> @@ -195,13 +195,15 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
>  	const struct tcf_connmark_info *ci = to_connmark(a);
>  	unsigned char *b = skb_tail_pointer(skb);
>  	const struct tcf_connmark_parms *parms;
> -	struct tc_connmark opt = {
> -		.index   = ci->tcf_index,
> -		.refcnt  = refcount_read(&ci->tcf_refcnt) - ref,
> -		.bindcnt = atomic_read(&ci->tcf_bindcnt) - bind,
> -	};
> +	struct tc_connmark opt;
>  	struct tcf_t t;
>  
> +	memset(&opt, 0, sizeof(opt));
> +
> +	index   = ci->tcf_index;
> +	refcnt  = refcount_read(&ci->tcf_refcnt) - ref;
> +	bindcnt = atomic_read(&ci->tcf_bindcnt) - bind;

I think some editing errors have crept in here,
because the above does not compile: index should be opt.index, ...

> +
>  	rcu_read_lock();
>  	parms = rcu_dereference(ci->parms);

-- 
pw-bot: changes-requested