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

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

Posted by Edward Adam Davis 2 weeks, 1 day 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>
---
v3:
  - update coding style and practices
v2: https://lore.kernel.org/all/tencent_E83074AB763967783C9D36949674363C4A09@qq.com/
  - 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..423503d2e7a7 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)
+{
+	u32 sizeoftlvs;
+	struct atmlec_msg *mesg;
+	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 && !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

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

Posted by Simon Horman 1 week, 2 days ago

+ Dharanitharan

On Thu, Dec 04, 2025 at 07:17:22PM +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].
> 
> 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>
> ---
> v3:
>   - update coding style and practices
> v2: https://lore.kernel.org/all/tencent_E83074AB763967783C9D36949674363C4A09@qq.com/
>   - update subject and comments for pre_send
> v1: https://lore.kernel.org/all/tencent_B31D1B432549BA28BB5633CB9E2C1B124B08@qq.com

FTR, a similar patch has been posted by Dharanitharan (CCed)

- [PATCH v3] net: atm: lec: add pre_send validation to avoid uninitialized
  https://lore.kernel.org/all/20251210035354.17492-2-dharanitharan725@gmail.com/

The main difference between that patch and this one is
a check for msg_size being present in linear data.

I would appreciate some collaboration between the authors of these patches.

Thanks!

-- 
pw-bot: changes-requested

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

Posted by Edward Adam Davis 1 week, 2 days ago

Sun, Wed, 10 Dec 2025 10:31:34 +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].
> >
> > 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>
> > ---
> > v3:
> >   - update coding style and practices
> > v2: https://lore.kernel.org/all/tencent_E83074AB763967783C9D36949674363C4A09@qq.com/
> >   - update subject and comments for pre_send
> > v1: https://lore.kernel.org/all/tencent_B31D1B432549BA28BB5633CB9E2C1B124B08@qq.com
> 
> FTR, a similar patch has been posted by Dharanitharan (CCed)
Didn't you check the dates? I released the third version of the patch
on December 4th (the first version was on November 28th), while this
person above released their first version of the patch on December 7th.
Their patch is far too similar to mine!

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

Posted by Simon Horman 1 week, 2 days ago

On Wed, Dec 10, 2025 at 06:50:02PM +0800, Edward Adam Davis wrote:
> Sun, Wed, 10 Dec 2025 10:31:34 +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].
> > >
> > > 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>
> > > ---
> > > v3:
> > >   - update coding style and practices
> > > v2: https://lore.kernel.org/all/tencent_E83074AB763967783C9D36949674363C4A09@qq.com/
> > >   - update subject and comments for pre_send
> > > v1: https://lore.kernel.org/all/tencent_B31D1B432549BA28BB5633CB9E2C1B124B08@qq.com
> > 
> > FTR, a similar patch has been posted by Dharanitharan (CCed)
> Didn't you check the dates? I released the third version of the patch
> on December 4th (the first version was on November 28th), while this
> person above released their first version of the patch on December 7th.
> Their patch is far too similar to mine!

Yes, I was aware of the timeline when I wrote my previous email.

My preference is for some consensus to be reached on the way forward:
both technically and in terms of process.

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

Posted by Edward Adam Davis 1 week, 1 day ago

On Wed, 10 Dec 2025 13:02:56 +0000, Simon Horman wrote:
> On Wed, Dec 10, 2025 at 06:50:02PM +0800, Edward Adam Davis wrote:
> > Sun, Wed, 10 Dec 2025 10:31:34 +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].
> > > >
> > > > 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>
> > > > ---
> > > > v3:
> > > >   - update coding style and practices
> > > > v2: https://lore.kernel.org/all/tencent_E83074AB763967783C9D36949674363C4A09@qq.com/
> > > >   - update subject and comments for pre_send
> > > > v1: https://lore.kernel.org/all/tencent_B31D1B432549BA28BB5633CB9E2C1B124B08@qq.com
> > >
> > > FTR, a similar patch has been posted by Dharanitharan (CCed)
> > Didn't you check the dates? I released the third version of the patch
> > on December 4th (the first version was on November 28th), while this
> > person above released their first version of the patch on December 7th.
> > Their patch is far too similar to mine!
> 
> Yes, I was aware of the timeline when I wrote my previous email.
> 
> My preference is for some consensus to be reached on the way forward:
> both technically and in terms of process.
I'm a little confused. Why are you explaining the process to someone
who submitted a patch 99% similar to mine, just a few days after I did?

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

Posted by Simon Horman 1 week, 1 day ago

On Thu, Dec 11, 2025 at 02:55:45PM +0800, Edward Adam Davis wrote:
> On Wed, 10 Dec 2025 13:02:56 +0000, Simon Horman wrote:
> > On Wed, Dec 10, 2025 at 06:50:02PM +0800, Edward Adam Davis wrote:
> > > Sun, Wed, 10 Dec 2025 10:31:34 +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].
> > > > >
> > > > > 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>
> > > > > ---
> > > > > v3:
> > > > >   - update coding style and practices
> > > > > v2: https://lore.kernel.org/all/tencent_E83074AB763967783C9D36949674363C4A09@qq.com/
> > > > >   - update subject and comments for pre_send
> > > > > v1: https://lore.kernel.org/all/tencent_B31D1B432549BA28BB5633CB9E2C1B124B08@qq.com
> > > >
> > > > FTR, a similar patch has been posted by Dharanitharan (CCed)
> > > Didn't you check the dates? I released the third version of the patch
> > > on December 4th (the first version was on November 28th), while this
> > > person above released their first version of the patch on December 7th.
> > > Their patch is far too similar to mine!
> > 
> > Yes, I was aware of the timeline when I wrote my previous email.
> > 
> > My preference is for some consensus to be reached on the way forward:
> > both technically and in terms of process.
> I'm a little confused. Why are you explaining the process to someone
> who submitted a patch 99% similar to mine, just a few days after I did?

It's always tricky when similar patches are on the ML on the same time.
Ultimately what I would like is for a correct solution to be merged.
Ideally in a way that makes everyone happy.

I'm explaining that to everyone: in this thread, and elsewhere.