net: atm: targetless need more input msg

[PATCH] net: atm: targetless need more input msg

Posted by Edward Adam Davis 3 days, 5 hours ago

syzbot found an uninitialized targetless variable. The user-provided
data was only 28 bytes long, but initializing targetless requires at
least 44 bytes. This discrepancy ultimately led to the uninitialized
variable access issue reported by syzbot [1].

Adding a message length check to the arp update process eliminates
the uninitialized issue in [1].

[1]
BUG: KMSAN: uninit-value in lec_arp_update net/atm/lec.c:1845 [inline]
 lec_arp_update net/atm/lec.c:1845 [inline]
 lec_atm_send+0x2b02/0x55b0 net/atm/lec.c:385
 vcc_sendmsg+0x1052/0x1190 net/atm/common.c:650

Reported-by: syzbot+5dd615f890ddada54057@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 net/atm/lec.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index afb8d3eb2185..178132b2771a 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -382,6 +382,15 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 			break;
 		fallthrough;
 	case l_arp_update:
+	{
+		int need_size = offsetofend(struct atmlec_msg,
+				content.normal.targetless_le_arp);
+		if (skb->len < need_size) {
+			pr_info("Input msg size too small, need %d got %u\n",
+				 need_size, skb->len);
+			dev_kfree_skb(skb);
+			return -EINVAL;
+		}
 		lec_arp_update(priv, mesg->content.normal.mac_addr,
 			       mesg->content.normal.atm_addr,
 			       mesg->content.normal.flag,
@@ -394,6 +403,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 					    tmp, mesg->sizeoftlvs);
 		}
 		break;
+	}
 	case l_config:
 		priv->maximum_unknown_frame_count =
 		    mesg->content.config.maximum_unknown_frame_count;
-- 
2.43.0

Re: [PATCH] net: atm: targetless need more input msg

Posted by Simon Horman 1 day, 5 hours ago

Hi Edward,

Thanks for taking time to look into this issue.

On Fri, Nov 28, 2025 at 11:56:25PM +0800, Edward Adam Davis wrote:
> syzbot found an uninitialized targetless variable. The user-provided
> data was only 28 bytes long, but initializing targetless requires at
> least 44 bytes. This discrepancy ultimately led to the uninitialized
> variable access issue reported by syzbot [1].
> 
> Adding a message length check to the arp update process eliminates
> the uninitialized issue in [1].
> 
> [1]
> BUG: KMSAN: uninit-value in lec_arp_update net/atm/lec.c:1845 [inline]
>  lec_arp_update net/atm/lec.c:1845 [inline]
>  lec_atm_send+0x2b02/0x55b0 net/atm/lec.c:385
>  vcc_sendmsg+0x1052/0x1190 net/atm/common.c:650
> 
> Reported-by: syzbot+5dd615f890ddada54057@syzkaller.appspotmail.com

I think it would be useful to also include:

Closes: https://syzkaller.appspot.com/bug?extid=5dd615f890ddada54057

And as a fix for Networking code it should include a fixes tag.
Briefly examining the history of this code, using git annotate,
it seems that this problem has existed since the beginning of git history.
If so, this tag seems appropriate:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

Also, as a fix for Networking code present in the net tree,
it should be targeted at that tree, like this:

Subject: [PATCH net] ...

More information on the Networking development workflow can be found here:
https://docs.kernel.org/process/maintainer-netdev.html

> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  net/atm/lec.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index afb8d3eb2185..178132b2771a 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -382,6 +382,15 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
>  			break;
>  		fallthrough;
>  	case l_arp_update:
> +	{
> +		int need_size = offsetofend(struct atmlec_msg,
> +				content.normal.targetless_le_arp);
> +		if (skb->len < need_size) {

As per Eric's comment on a similar fix [1],
you should probably be using pskb_may_pull().

Also, I see that this patch addresses the l_arp_update case.
But it looks like a similar problem exist in least in the l_config case
too.

So I think it would be useful take a more holistic approach.
Perhaps in the form of a patchset if you want to restrict this
patch to addressing the specific problem flagged by syzbot.

[1] https://lore.kernel.org/netdev/20251126034601.236922-1-ssranevjti@gmail.com/

> +			pr_info("Input msg size too small, need %d got %u\n",
> +				 need_size, skb->len);
> +			dev_kfree_skb(skb);
> +			return -EINVAL;
> +		}
>  		lec_arp_update(priv, mesg->content.normal.mac_addr,
>  			       mesg->content.normal.atm_addr,
>  			       mesg->content.normal.flag,

-- 
pw-bot: changes-requested

Re: [PATCH] net: atm: targetless need more input msg

Posted by Edward Adam Davis 19 hours ago

Sun, 30 Nov 2025 15:56:42 +0000, Simon Horman wrote:
> > syzbot found an uninitialized targetless variable. The user-provided
> > data was only 28 bytes long, but initializing targetless requires at
> > least 44 bytes. This discrepancy ultimately led to the uninitialized
> > variable access issue reported by syzbot [1].
> > 
> > Adding a message length check to the arp update process eliminates
> > the uninitialized issue in [1].
> > 
> > [1]
> > BUG: KMSAN: uninit-value in lec_arp_update net/atm/lec.c:1845 [inline]
> >  lec_arp_update net/atm/lec.c:1845 [inline]
> >  lec_atm_send+0x2b02/0x55b0 net/atm/lec.c:385
> >  vcc_sendmsg+0x1052/0x1190 net/atm/common.c:650
> > 
> > Reported-by: syzbot+5dd615f890ddada54057@syzkaller.appspotmail.com
> 
> I think it would be useful to also include:
> 
> Closes: https://syzkaller.appspot.com/bug?extid=5dd615f890ddada54057
> 
> And as a fix for Networking code it should include a fixes tag.
> Briefly examining the history of this code, using git annotate,
> it seems that this problem has existed since the beginning of git history.
> If so, this tag seems appropriate:
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 
> Also, as a fix for Networking code present in the net tree,
> it should be targeted at that tree, like this:
> 
> Subject: [PATCH net] ...
Thanks very much for your oppinions. I will send v2 for it.
> 
> More information on the Networking development workflow can be found here:
> https://docs.kernel.org/process/maintainer-netdev.html
> 
> 
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> >  net/atm/lec.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/net/atm/lec.c b/net/atm/lec.c
> > index afb8d3eb2185..178132b2771a 100644
> > --- a/net/atm/lec.c
> > +++ b/net/atm/lec.c
> > @@ -382,6 +382,15 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
> >  			break;
> >  		fallthrough;
> >  	case l_arp_update:
> > +	{
> > +		int need_size = offsetofend(struct atmlec_msg,
> > +				content.normal.targetless_le_arp);
> > +		if (skb->len < need_size) {
> 
> As per Eric's comment on a similar fix [1],
> you should probably be using pskb_may_pull().
The pskb_may_pull() function performs a more thorough check of the skb
length, which is especially suitable for handling non-linear data areas.
> 
> Also, I see that this patch addresses the l_arp_update case.
> But it looks like a similar problem exist in least in the l_config case
> too.
I noticed this, and it's not just these; several types of atmlec_msg
handled in lec_atm_send() are also involved.
Strictly speaking, they all require relevant checks.
> 
> So I think it would be useful take a more holistic approach.
> Perhaps in the form of a patchset if you want to restrict this
> patch to addressing the specific problem flagged by syzbot.
Okay, I'll carefully consider how to handle the others.
> 
> [1] https://lore.kernel.org/netdev/20251126034601.236922-1-ssranevjti@gmail.com/
> 
> > +			pr_info("Input msg size too small, need %d got %u\n",
> > +				 need_size, skb->len);
> > +			dev_kfree_skb(skb);
> > +			return -EINVAL;
> > +		}
> >  		lec_arp_update(priv, mesg->content.normal.mac_addr,
> >  			       mesg->content.normal.atm_addr,
> >  			       mesg->content.normal.flag,

[PATCH net v2] net: atm: implement pre_send to check input before sending

Posted by Edward Adam Davis 17 hours ago

syzbot found an uninitialized targetless variable. The user-provided
data was only 28 bytes long, but initializing targetless requires at
least 44 bytes. This discrepancy ultimately led to the uninitialized
variable access issue reported by syzbot [1].

Besides the issues reported by syzbot regarding targetless messages
[1], similar problems exist in other types of messages as well. We will
uniformly add input data checks to pre_send to prevent uninitialized
issues from recurring.

Additionally, for cases where sizeoftlvs is greater than 0, the skb
requires more memory, and this will also be checked.

[1]
BUG: KMSAN: uninit-value in lec_arp_update net/atm/lec.c:1845 [inline]
 lec_arp_update net/atm/lec.c:1845 [inline]
 lec_atm_send+0x2b02/0x55b0 net/atm/lec.c:385
 vcc_sendmsg+0x1052/0x1190 net/atm/common.c:650

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+5dd615f890ddada54057@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5dd615f890ddada54057
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v2:
  - update subject and comments for pre_send
v1: https://lore.kernel.org/all/tencent_B31D1B432549BA28BB5633CB9E2C1B124B08@qq.com

 net/atm/lec.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index afb8d3eb2185..8a9660abd134 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -340,6 +340,23 @@ static int lec_close(struct net_device *dev)
 	return 0;
 }
 
+static int lec_atm_pre_send(struct atm_vcc *vcc, struct sk_buff *skb)
+{
+	struct atmlec_msg *mesg;
+	int sizeoftlvs;
+	int msg_size = sizeof(struct atmlec_msg);
+
+	if (skb->len < msg_size)
+		return -EINVAL;
+
+	mesg = (struct atmlec_msg *)skb->data;
+	sizeoftlvs = mesg->sizeoftlvs;
+	if (sizeoftlvs > 0 && !pskb_may_pull(skb, msg_size + sizeoftlvs))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 {
 	static const u8 zero_addr[ETH_ALEN] = {};
@@ -491,6 +508,7 @@ static void lec_atm_close(struct atm_vcc *vcc)
 
 static const struct atmdev_ops lecdev_ops = {
 	.close = lec_atm_close,
+	.pre_send = lec_atm_pre_send,
 	.send = lec_atm_send
 };
 
-- 
2.43.0