atm: atmtcp: Free invalid length skb in atmtcp_c_send().
Subsystems (3)
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
@@ -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
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 tovcc->dev->ops->send().
Quoted 6 lines
atmtcp_c_send()accessesskb->dataas structatmtcp_hdrafter checking ifskb->lenis 0, but it's not enough.Also, when
skb->len== 0, skb and sk (vcc) were leaked becausedev_kfree_skb()is not called andatm_return()is missing to revertatm_account_tx()invcc_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
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Reported-by [email protected]Tested-by [email protected]Signed-off-by Kuniyuki Iwashima <[email protected]>
My question above not withstanding, this looks good to me.
...
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 tovcc->dev->ops->send().
atmtcp_c_send()accessesskb->dataas structatmtcp_hdrafter checking ifskb->lenis 0, but it's not enough.
Quoted 8 lines
Quoted 3 lines
Also, when
skb->len== 0, skb and sk (vcc) were leaked becausedev_kfree_skb()is not called andatm_return()is missing to revertatm_account_tx()invcc_sendmsg().Hi Iwashima-san,
I agree with the above and your patch.
But I am wondering ifatm_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_sendreturns-ENOBUFSbecauseatm_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_fullreturns false invcc_sendmsg.
I agree. I can send a followup.
Thank you!
pw-bot: cr