Re: [PATCH net v1] atm: lec: fix null-ptr-deref in lec_arp_clear_vccs

[Jiayuan Chen]

February 25, 2026 at 17:45, "Simon Horman" <horms@xxxxxxxxxx mailto:horms@xxxxxxxxxx?to=%22Simon%20Horman%22%20%3Chorms%40kernel.org%3E > wrote:


> 
> On Wed, Feb 25, 2026 at 08:37:05AM +0000, Simon Horman wrote:
> 
> > 
> > On Tue, Feb 24, 2026 at 12:46:38PM +0800, Jiayuan Chen wrote:
> >  From: Jiayuan Chen <jiayuan.chen@xxxxxxxxxx>
> >  
> >  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")
> 
> > 
> > Reported-by: syzbot+72e3ea390c305de0e259@xxxxxxxxxxxxxxxxxxxxxxxxx
> >  Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@xxxxxxxxxx/T/
> >  Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >  Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxxx>
> >  
> >  Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
> >
>