Re: [PATCH net v1] atm: lec: fix null-ptr-deref in lec_arp_clear_vccs
From: "Jiayuan Chen" <jiayuan.chen@linux.dev>
Date: 2026-02-25 10:16:44
Also in:
lkml
February 25, 2026 at 17:45, "Simon Horman" <horms@kernel.org mailto:horms@kernel.org?to=%22Simon%20Horman%22%20%3Chorms%40kernel.org%3E > wrote:
On Wed, Feb 25, 2026 at 08:37:05AM +0000, Simon Horman wrote:quoted
On Tue, Feb 24, 2026 at 12:46:38PM +0800, Jiayuan Chen wrote: From: Jiayuan Chen [off-list ref] syzkaller reported a null-ptr-deref in lec_arp_clear_vccs(). This issue can be easily reproduced using the syzkaller reproducer. In the ATM LANE (LAN Emulation) module, the same atm_vcc can be shared by multiple lec_arp_table entries (e.g., via entry->vcc or entry->recv_vcc). When the underlying VCC is closed, lec_vcc_close() iterates over all ARP entries and calls lec_arp_clear_vccs() for each matched entry. For example, when lec_vcc_close() iterates through the hlists in priv->lec_arp_empty_ones or other ARP tables: 1. In the first iteration, for the first matched ARP entry sharing the VCC, lec_arp_clear_vccs() frees the associated vpriv (which is vcc->user_back) and sets vcc->user_back to NULL. 2. In the second iteration, for the next matched ARP entry sharing the same VCC, lec_arp_clear_vccs() is called again. It obtains a NULL vpriv from vcc->user_back (via LEC_VCC_PRIV(vcc)) and then attempts to dereference it via `vcc->pop = vpriv->old_pop`, leading to a null-ptr-deref crash. Fix this by adding a null check for vpriv before dereferencing it. If vpriv is already NULL, it means the VCC has been cleared by a previous call, so we can safely skip the cleanup and just clear the entry's vcc/recv_vcc pointers. Note that the added check is intentional and necessary to avoid calling vcc_release_async() multiple times on the same vcc/recv_vcc, not just protecting the kfree().Sorry for coming back to this a 2nd time. After thinking about this some more I'd like to pass on some feedback from the AI powered review. I'll put the full text below. But in a nutshell: could you clarify why it is necessary to avoid calling vcc_release_async() multiple times.
Hi Simon,
Thanks for the feedback.
1. Regarding why it is necessary to avoid calling vcc_release_async() multiple times:
when vpriv is NULL, it means a previous call to lec_arp_clear_vccs() has already freed vpriv, set
vcc->user_back = NULL, restored vcc->push, and called vcc_release_async() for this VCC.
Calling vcc_release_async() again on an already-released VCC would redundantly set flags, set
sk_err, and trigger sk_state_change() on a socket that is already shutting down. While this may not
cause an immediate crash, it is semantically wrong — the VCC has already been properly
released, and we should not repeat the teardown sequence.
That is why I intentionally placed the entire cleanup block (including vcc_release_async()) inside
the if (vpriv) guard, rather than only guarding the vpriv dereference. The NULL vpriv
serves as a reliable indicator that this VCC has already been processed by a prior iteration.
2. Regarding the Fixes tag: the AI suggests pointing to 8d9f73c0ad2f ("atm: fix a memory leak of vcc->user_back"),
but that only introduced the entry->recv_vcc cleanup path. The entry->vcc path has had the same bug since the
beginning — if two ARP entries share the same VCC, the second call dereferences NULL vpriv via vcc->pop = vpriv->old_pop.
My patch fixes both paths, and the entry->vcc path bug traces back to the original code,
so Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") is correct. I've verified that the entry->vcc path is
independently triggerable with a seperated reproducer [1] (it's not worth to put it into selftest for legacy module)
I also don't think splitting this into two patches makes sense — both paths are in the same function,
exhibit the same bug pattern, and the fix is identical.
Best,
Jiayuan
[1]:
// This demonstrates that the entry->vcc NULL dereference bug has existed
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <linux/atm.h>
#include <linux/atmdev.h>
#include <linux/atmlec.h>
int main(void)
{
int fd_lecd, fd_data;
struct atmlec_ioc ioc_data;
fd_lecd = socket(AF_ATMPVC, SOCK_DGRAM, 0);
if (fd_lecd < 0) {
perror("socket(AF_ATMPVC) for lecd");
return 1;
}
if (ioctl(fd_lecd, ATMLEC_CTRL, 0) < 0) {
perror("ioctl(ATMLEC_CTRL)");
close(fd_lecd);
return 1;
}
printf("LEC device 0 initialized (lec0)\n");
fd_data = socket(AF_ATMPVC, SOCK_DGRAM, 0);
if (fd_data < 0) {
perror("socket(AF_ATMPVC) for data");
close(fd_lecd);
return 1;
}
memset(&ioc_data, 0, sizeof(ioc_data));
ioc_data.dev_num = 0;
ioc_data.receive = 0; /* send VCC -> entry->vcc path */
printf("Calling ATMLEC_DATA (1st time, receive=0 -> entry->vcc path)...\n");
if (ioctl(fd_data, ATMLEC_DATA, &ioc_data) < 0) {
perror("ioctl(ATMLEC_DATA) #1");
close(fd_data);
close(fd_lecd);
return 1;
}
printf("Calling ATMLEC_DATA (2nd time, same VCC -> 2nd entry in empty_ones)...\n");
if (ioctl(fd_data, ATMLEC_DATA, &ioc_data) < 0) {
perror("ioctl(ATMLEC_DATA) #2");
close(fd_data);
close(fd_lecd);
return 1;
}
printf("Closing data VCC socket (will trigger NULL deref in entry->vcc path)...\n");
close(fd_data);
printf("If you see this, the bug was not triggered (maybe already fixed?)\n");
close(fd_lecd);
return 0;
}
panic:
[ 42.814407] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 42.814431] #PF: supervisor read access in kernel mode
[ 42.814433] #PF: error_code(0x0000) - not-present page
[ 42.814435] PGD 0 P4D 0
[ 42.814438] Oops: Oops: 0000 [#1] SMP PTI
[ 42.814455] CPU: 1 UID: 0 PID: 452 Comm: repro_lec_vcc Not tainted 6.19.0+ #175 PREEMPT_RT
[ 42.814460] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 42.814464] RIP: 0010:lec_arp_clear_vccs+0x2a/0xe0
[ 42.814532] Code: 0f 1f 44 00 00 55 48 89 e5 41 55 41 54 53 4c 8b 67 30 48 89 fb 4d 85 e4 74 5d 4d 8b ac 24 d0 08 00 00 49 8b 94 24 50 08 00 00 <49> 8b 45 00 49 89 84 24 30 08 00 00 41 80
[ 42.814544] RSP: 0018:ffffd1674176bca0 EFLAGS: 00010286
[ 42.814546] RAX: 0000000000000000 RBX: ffff89faca9c1800 RCX: 0000000000000000
[ 42.814548] RDX: ffff89facc23a000 RSI: 0000000000000000 RDI: ffff89faca9c1800
[ 42.814550] RBP: ffffd1674176bcb8 R08: 0000000000000000 R09: 0000000000000000
[ 42.814551] R10: 0000000000000000 R11: 0000000000000000 R12: ffff89facc10b000
[ 42.814553] R13: 0000000000000000 R14: dead000000000100 R15: ffff89facc10b000
[ 42.814554] FS: 00007fb608daa740(0000) GS:ffff89fb536fb000(0000) knlGS:0000000000000000
[ 42.814557] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 42.814558] CR2: 0000000000000000 CR3: 000000010a8f0006 CR4: 0000000000770ef0
[ 42.814563] PKRU: 55555554
[ 42.814568] Call Trace:
[ 42.814570] <TASK>
[ 42.814573] lec_push+0x33e/0x7a0
[ 42.814583] vcc_release+0x72/0x210
[ 42.814588] __sock_release+0x40/0xc0
[ 42.814636] sock_close+0x18/0x30
[ 42.814639] __fput+0x114/0x310
[ 42.814690] fput_close_sync+0x3d/0xa0
[ 42.814693] __x64_sys_close+0x3e/0x90
[ 42.814707] x64_sys_call+0x1b7c/0x26e0
[ 42.814748] do_syscall_64+0xd3/0x1510
[ 42.814801] ? find_held_lock+0x31/0x90
[ 42.814838] ? exc_page_fault+0x94/0x260
[ 42.814852] ? lock_release+0xcd/0x280
[ 42.814855] ? handle_mm_fault+0x1ea/0x300
[ 42.814884] ? irqentry_exit+0x17f/0x780
[ 42.814890] ? clear_bhb_loop+0x30/0x80
[ 42.814937] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 42.814939] RIP: 0033:0x7fb608ec3724
[ 42.814981] Code: 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 80 3d 25 49 0f 00 00 74 13 b8 03 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 3c c3 0f 1f 00 55 4d
[ 42.814983] RSP: 002b:00007ffdaeb3aeb8 EFLAGS: 00000202 ORIG_RAX: 0000000000000003
[ 42.814986] RAX: ffffffffffffffda RBX: 00007ffdaeb3b028 RCX: 00007fb608ec3724
[ 42.814988] RDX: 0000000000000000 RSI: 00005609f3c8f2a0 RDI: 0000000000000004
[ 42.814989] RBP: 00007ffdaeb3af00 R08: 00007fb608fb0b20 R09: 0000000000000000
[ 42.814990] R10: 0000000000000001 R11: 0000000000000202 R12: 0000000000000001
[ 42.814991] R13: 0000000000000000 R14: 00005609ce2cad90 R15: 00007fb609005000
[ 42.815009] </TASK>
[ 42.815010] Modules linked in:
[ 42.815065] CR2: 0000000000000000
[ 42.815066] ---[ end trace 0000000000000000 ]---
[ 42.815079] RIP: 0010:lec_arp_clear_vccs+0x2a/0xe0
[ 42.815082] Code: 0f 1f 44 00 00 55 48 89 e5 41 55 41 54 53 4c 8b 67 30 48 89 fb 4d 85 e4 74 5d 4d 8b ac 24 d0 08 00 00 49 8b 94 24 50 08 00 00 <49> 8b 45 00 49 89 84 24 30 08 00 00 41 80
[ 42.815084] RSP: 0018:ffffd1674176bca0 EFLAGS: 00010286
[ 42.815086] RAX: 0000000000000000 RBX: ffff89faca9c1800 RCX: 0000000000000000
[ 42.815087] RDX: ffff89facc23a000 RSI: 0000000000000000 RDI: ffff89faca9c1800
[ 42.815088] RBP: ffffd1674176bcb8 R08: 0000000000000000 R09: 0000000000000000
[ 42.815090] R10: 0000000000000000 R11: 0000000000000000 R12: ffff89facc10b000
[ 42.815100] R13: 0000000000000000 R14: dead000000000100 R15: ffff89facc10b000
[ 42.815105] FS: 00007fb608daa740(0000) GS:ffff89fb536fb000(0000) knlGS:0000000000000000
[ 42.815107] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 42.815108] CR2: 0000000000000000 CR3: 000000010a8f0006 CR4: 0000000000770ef0
[ 42.815112] PKRU: 55555554
[ 42.815113] note: repro_lec_vcc[452] exited with irqs disabled
[ 42.815116] BUG: sleeping function called from invalid context at kernel/locking/rtmutex_api.c:532
[ 42.815118] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 452, name: repro_lec_vcc
[ 42.815120] preempt_count: 0, expected: 0
[ 42.815131] RCU nest depth: 1, expected: 0
[ 42.815132] INFO: lockdep is turned off.
[ 42.815134] CPU: 1 UID: 0 PID: 452 Comm: repro_lec_vcc Tainted: G D 6.19.0+ #175 PREEMPT_RT
[ 42.815138] Tainted: [D]=DIE
[ 42.815139] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 42.815140] Call Trace:
[ 42.815142] <TASK>
[ 42.815143] dump_stack_lvl+0xc1/0xf0
[ 42.815178] dump_stack+0x10/0x20
[ 42.815180] __might_resched+0x184/0x270
[ 42.815206] ? sched_mm_cid_exit+0x5e/0x1f0
[ 42.815219] __might_sleep+0x49/0x60
[ 42.815223] mutex_lock_nested+0x2f/0xc0
[ 42.815238] sched_mm_cid_exit+0x5e/0x1f0
[ 42.815243] do_exit+0xb6/0xac0
[ 42.815274] make_task_dead+0x94/0x180
[ 42.815278] rewind_stack_and_make_dead+0x16/0x20
[ 42.815281] RIP: 0033:0x7fb608ec3724
[ 42.815284] Code: 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 80 3d 25 49 0f 00 00 74 13 b8 03 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 3c c3 0f 1f 00 55 4d
[ 42.815285] RSP: 002b:00007ffdaeb3aeb8 EFLAGS: 00000202 ORIG_RAX: 0000000000000003
[ 42.815287] RAX: ffffffffffffffda RBX: 00007ffdaeb3b028 RCX: 00007fb608ec3724
[ 42.815289] RDX: 0000000000000000 RSI: 00005609f3c8f2a0 RDI: 0000000000000004
[ 42.815290] RBP: 00007ffdaeb3af00 R08: 00007fb608fb0b20 R09: 0000000000000000
[ 42.815291] R10: 0000000000000001 R11: 0000000000000202 R12: 0000000000000001
[ 42.815292] R13: 0000000000000000 R14: 00005609ce2cad90 R15: 00007fb609005000
[ 42.815302] </TASK>
AI output was: Is this description of the guard's purpose accurate? The commit message states that the null check is "necessary to avoid calling vcc_release_async() multiple times on the same vcc/recv_vcc, not just protecting the kfree()." Looking at vcc_release_async() in net/atm/common.c, it sets ATM_VF_CLOSE flag, sets sk_shutdown |= RCV_SHUTDOWN, sets sk_err, clears ATM_VF_WAITING, and calls sk_state_change(sk). Calling it multiple times would redundantly set flags that are already set and trigger sk_state_change() multiple times on an already-shutting-down socket. While preventing multiple vcc_release_async() calls is beneficial, the guard is primarily needed to prevent the NULL dereference on vpriv->old_pop (which would crash) and secondarily to prevent use-after-free on the kfree'd vpriv. None of the repeated vcc_release_async() operations would necessarily cause a crash or data corruption -- they would just set already-set flags and trigger redundant callbacks. Could the commit message be more precise about the primary purpose of the guard being to prevent the NULL dereference, with preventing multiple vcc_release_async() calls being a beneficial side effect rather than the main reason for the check? And, I believe due to that, AI goes on to comment about the fixes tag. FWIIW, I think the commit you cited is correct with respect to the first part of your patch which protecting against double-free. But I do begin to wonder if we may have two fixes in one patch. Should the Fixes tag point to a more recent commit? The null pointer dereference bug was directly introduced by commit 8d9f73c0ad2f ("atm: fix a memory leak of vcc->user_back") in 2020. That commit added cleanup code for entry->recv_vcc that frees vpriv and sets vcc->user_back to NULL without checking if vpriv is already NULL. When multiple ARP entries share the same VCC, the second call to lec_arp_clear_vccs() dereferences a NULL vpriv, causing the crash. While the entry->vcc path had similar code since 2005, commit 8d9f73c0ad2f introduced the recv_vcc path with the same vulnerability, making the bug exploitable. Consider: Fixes: 8d9f73c0ad2f ("atm: fix a memory leak of vcc->user_back")quoted
Reported-by: syzbot+72e3ea390c305de0e259@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@google.com/T/ (local) Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Jiayuan Chen [off-list ref] Reviewed-by: Simon Horman [off-list ref]