Skip to contentSkip to conversation

atm: atmtcp: Free invalid length skb in atmtcp_c_send().

PATCH Superseded Kuniyuki Iwashima 2 participants 3 messages Jun 13, 2025 – Jun 14, 2025

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 atm: atmtcp: Free invalid length skb in atmtcp_c_send().

From: Kuniyuki Iwashima <[email protected]>

syzbot reported the splat below. [0]

vcc_sendmsg() copies data passed from userspace to skb and passes it to vcc->dev->ops->send().

atmtcp_c_send() accesses skb->data as struct atmtcp_hdr after checking if skb->len is 0, but it's not enough.

Also, when skb->len == 0, skb and sk (vcc) were leaked because dev_kfree_skb() is not called and atm_return() is missing to revert atm_account_tx() in vcc_sendmsg().

Let's properly free skb with an invalid length in atmtcp_c_send().

[0]:

BUG: KMSAN: uninit-value in atmtcp_c_send+0x255/0xed0 drivers/atm/atmtcp.c:294
 atmtcp_c_send+0x255/0xed0 drivers/atm/atmtcp.c:294
 vcc_sendmsg+0xd7c/0xff0 net/atm/common.c:644
 sock_sendmsg_nosec net/socket.c:712 [inline]
 __sock_sendmsg+0x330/0x3d0 net/socket.c:727
 ____sys_sendmsg+0x7e0/0xd80 net/socket.c:2566
 ___sys_sendmsg+0x271/0x3b0 net/socket.c:2620
 __sys_sendmsg net/socket.c:2652 [inline]
 __do_sys_sendmsg net/socket.c:2657 [inline]
 __se_sys_sendmsg net/socket.c:2655 [inline]
 __x64_sys_sendmsg+0x211/0x3e0 net/socket.c:2655
 x64_sys_call+0x32fb/0x3db0 arch/x86/include/generated/asm/syscalls_64.h:47
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xd9/0x210 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Uninit was created at:

 slab_post_alloc_hook mm/slub.c:4154 [inline]
 slab_alloc_node mm/slub.c:4197 [inline]
 kmem_cache_alloc_node_noprof+0x818/0xf00 mm/slub.c:4249
 kmalloc_reserve+0x13c/0x4b0 net/core/skbuff.c:579
 __alloc_skb+0x347/0x7d0 net/core/skbuff.c:670
 alloc_skb include/linux/skbuff.h:1336 [inline]
 vcc_sendmsg+0xb40/0xff0 net/atm/common.c:628
 sock_sendmsg_nosec net/socket.c:712 [inline]
 __sock_sendmsg+0x330/0x3d0 net/socket.c:727
 ____sys_sendmsg+0x7e0/0xd80 net/socket.c:2566
 ___sys_sendmsg+0x271/0x3b0 net/socket.c:2620
 __sys_sendmsg net/socket.c:2652 [inline]
 __do_sys_sendmsg net/socket.c:2657 [inline]
 __se_sys_sendmsg net/socket.c:2655 [inline]
 __x64_sys_sendmsg+0x211/0x3e0 net/socket.c:2655
 x64_sys_call+0x32fb/0x3db0 arch/x86/include/generated/asm/syscalls_64.h:47
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xd9/0x210 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

CPU: 1 UID: 0 PID: 5798 Comm: syz-executor192 Not tainted 6.16.0-rc1-syzkaller-00010-g2c4a1f3fe03e #0 PREEMPT(undef)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/07/2025
---
 drivers/atm/atmtcp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/atm/atmtcp.c +6 -1
index d4aa0f353b6c..54c6aeac63d0 100644
@@ -288,7 +288,12 @@ static int atmtcp_c_send(struct atm_vcc *vcc,struct sk_buff *skb)
 	struct sk_buff *new_skb;
 	int result = 0;
 
-	if (!skb->len) return 0;
+	if (skb->len < sizeof(struct atmtcp_hdr)) {
+		atm_return(vcc, skb->truesize);
+		dev_kfree_skb(skb);
+		return -EINVAL;
+	}
+
 	dev = vcc->dev_data;
 	hdr = (struct atmtcp_hdr *) skb->data;
 	if (hdr->length == ATMTCP_HDR_MAGIC) {
Signature · 1 line
2.49.0
reviewer Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · Kuniyuki Iwashima

From: Kuniyuki Iwashima <[email protected]>

syzbot reported the splat below. [0]

vcc_sendmsg() copies data passed from userspace to skb and passes it to vcc->dev->ops->send().

Quoted 6 lines

atmtcp_c_send() accesses skb->data as struct atmtcp_hdr after checking if skb->len is 0, but it's not enough.

Also, when skb->len == 0, skb and sk (vcc) were leaked because dev_kfree_skb() is not called and atm_return() is missing to revert atm_account_tx() in vcc_sendmsg().

Hi Iwashima-san,

I agree with the above and your patch.
But I am wondering if atm_return() also needs to be called when:

* atmtcp_c_send returns -ENOBUFS because atm_alloc_charge() fails.
* copy_from_iter_full returns false in vcc_sendmsg.

I ask because both occur after the call to atm_account_tx() in vcc_sendmsg().

Quoted 2 lines

Let's properly free skb with an invalid length in atmtcp_c_send().

...

Quoted 5 lines

My question above not withstanding, this looks good to me.

...

#2 lore raw

From: Simon Horman <[email protected]>
Date: Sat, 14 Jun 2025 17:19:59 +0100

Quoted 11 lines · Kuniyuki Iwashima
Quoted 10 lines

From: Kuniyuki Iwashima <[email protected]>

syzbot reported the splat below. [0]

vcc_sendmsg() copies data passed from userspace to skb and passes it to vcc->dev->ops->send().

atmtcp_c_send() accesses skb->data as struct atmtcp_hdr after checking if skb->len is 0, but it's not enough.

Quoted 8 lines
Quoted 3 lines

Also, when skb->len == 0, skb and sk (vcc) were leaked because dev_kfree_skb() is not called and atm_return() is missing to revert atm_account_tx() in vcc_sendmsg().

Hi Iwashima-san,

I agree with the above and your patch.
But I am wondering if atm_return() also needs to be called when:

Oh, I noticed atm_return() was for rmem_alloc, so I had to adjust wmem_alloc manually here.

Quoted 2 lines

* atmtcp_c_send returns -ENOBUFS because atm_alloc_charge() fails.

In this case, I guess vcc->pop is atm_pop_raw() that handles wmem accounting.

        if (vcc->pop) vcc->pop(vcc,skb);
        else dev_kfree_skb(skb);

If it's NULL, we need to adjust it here, but this pattern can be seen other ATM drivers..

Okay, sendmsg() requires SS_CONNECTED, and __vcc_connect() must go through one of atm_init_aal{0,34,5}, which sets ->pop to atm_pop_raw().

So, it seems !vcc->pop() is defensive unlikely path.

In v2, I'll change the invalid length handling to goto done;

Quoted 1 line

* copy_from_iter_full returns false in vcc_sendmsg.

I agree. I can send a followup.

Thank you!

pw-bot: cr