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

[Simon Horman <horms@kernel.org>]

+ 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@syzkaller.appspotmail.com
> Tested-by: syzbot+5dd615f890ddada54057@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5dd615f890ddada54057
> Signed-off-by: Dharanitharan R <dharanitharan725@gmail.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