linux-kernel - Re: [PATCH v3] net: atm: lec: add pre_send validation to avoid uninitialized

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: <aTlLZUDphWwZGaGS@horms.kernel.org>
Date: Wed, 10 Dec 2025 10:28:53 +0000
From: Simon Horman <horms@...nel.org>
To: Dharanitharan R <dharanitharan725@...il.com>
Cc: syzbot+5dd615f890ddada54057@...kaller.appspotmail.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	Edward Adam Davis <eadavis@...com>
Subject: Re: [PATCH v3] net: atm: lec: add pre_send validation to avoid
 uninitialized

+ Edward

On Wed, Dec 10, 2025 at 03:53:55AM +0000, Dharanitharan R wrote:
> syzbot reported a KMSAN uninitialized-value crash caused by reading
> fields from struct atmlec_msg before validating that the skb contains
> enough linear data. A malformed short skb can cause lec_arp_update()
> and other handlers to access uninitialized memory.
> 
> Add a pre_send() validator that ensures the message header and optional
> TLVs are fully present. This prevents all lec message types from reading
> beyond initialized skb data.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+5dd615f890ddada54057@...kaller.appspotmail.com
> Tested-by: syzbot+5dd615f890ddada54057@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5dd615f890ddada54057
> Signed-off-by: Dharanitharan R <dharanitharan725@...il.com>

As pointed out elsewhere, this patch largely duplicates the following by
Edward (CCed).

- [PATCH] net: atm: targetless need more input msg
  https://lore.kernel.org/netdev/tencent_B31D1B432549BA28BB5633CB9E2C1B124B08%40qq.com/

Ideally there would be some collaboration between you and him on this.

Also, as a fix for Networking code, please explicitly target the patch
at net-next.

Subject: [PATCH net] ...

> ---
>  net/atm/lec.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index afb8d3eb2185..c893781a490a 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -489,8 +489,33 @@ static void lec_atm_close(struct atm_vcc *vcc)
>  	module_put(THIS_MODULE);
>  }
>  
> +static int lec_atm_pre_send(struct atm_vcc *vcc, struct sk_buff *skb)
> +{
> +	struct atmlec_msg *mesg;
> +	u32 sizeoftlvs;
> +	unsigned int msg_size = sizeof(struct atmlec_msg);

Please use reverse xmas tree order - longest line to shortest -
for Networking code.

> +
> +	/* Must contain the base message */
> +	if (skb->len < msg_size)
> +		return -EINVAL;
> +
> +   	/* Must have at least msg_size bytes in linear data */
> +   	if (!pskb_may_pull(skb, msg_size))
> +   		return -EINVAL;

As you pointed out off-list, this check seems to be the main difference
between your patch and Edward's. I agree it seems relevant. But if so I
believe it duplicates the skb->len check above which can be dropped.

Also The indentation of the three above is not correct.
It should only use tabs.  checkpatch.pl points this out.

> +
> +	mesg = (struct atmlec_msg *)skb->data;
> +	sizeoftlvs = mesg->sizeoftlvs;
> +
> +	/* Validate TLVs if present */
> +	if (sizeoftlvs && !pskb_may_pull(skb, msg_size + sizeoftlvs))
> +       	return -EINVAL;

Tabs only for indentation on the like above too.

> +
> +	return 0;
> +}
> +
>  static const struct atmdev_ops lecdev_ops = {
>  	.close = lec_atm_close,
> +	.pre_send = lec_atm_pre_send,
>  	.send = lec_atm_send
>  };

-- 
pw-bot: changes requested