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

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

[Jiayuan Chen]

From: Jiayuan Chen <jiayuan.chen@shopee.com>

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().

Reported-by: syzbot+72e3ea390c305de0e259@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@google.com/T/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
---
 net/atm/lec.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index afb8d3eb2185..a5b80d6df603 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -1260,24 +1260,27 @@ static void lec_arp_clear_vccs(struct lec_arp_table *entry)
 		struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
 		struct net_device *dev = (struct net_device *)vcc->proto_data;
 
-		vcc->pop = vpriv->old_pop;
-		if (vpriv->xoff)
-			netif_wake_queue(dev);
-		kfree(vpriv);
-		vcc->user_back = NULL;
-		vcc->push = entry->old_push;
-		vcc_release_async(vcc, -EPIPE);
+		if (vpriv) {
+			vcc->pop = vpriv->old_pop;
+			if (vpriv->xoff)
+				netif_wake_queue(dev);
+			kfree(vpriv);
+			vcc->user_back = NULL;
+			vcc->push = entry->old_push;
+			vcc_release_async(vcc, -EPIPE);
+		}
 		entry->vcc = NULL;
 	}
 	if (entry->recv_vcc) {
 		struct atm_vcc *vcc = entry->recv_vcc;
 		struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
 
-		kfree(vpriv);
-		vcc->user_back = NULL;
-
-		entry->recv_vcc->push = entry->old_recv_push;
-		vcc_release_async(entry->recv_vcc, -EPIPE);
+		if (vpriv) {
+			kfree(vpriv);
+			vcc->user_back = NULL;
+			vcc->push = entry->old_recv_push;
+			vcc_release_async(vcc, -EPIPE);
+		}
 		entry->recv_vcc = NULL;
 	}
 }
-- 
2.43.0

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

[Simon Horman]

On Tue, Feb 24, 2026 at 12:46:38PM +0800, Jiayuan Chen wrote:
> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> 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().
> 
> Reported-by: syzbot+72e3ea390c305de0e259@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@google.com/T/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>

Reviewed-by: Simon Horman <horms@kernel.org>

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

[Simon Horman]

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@shopee.com>
> > 
> > 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.

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@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@google.com/T/
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> 

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@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:
> 
> > 
> > On Tue, Feb 24, 2026 at 12:46:38PM +0800, Jiayuan Chen wrote:
> >  From: Jiayuan Chen <jiayuan.chen@shopee.com>
> >  
> >  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@syzkaller.appspotmail.com
> >  Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@google.com/T/
> >  Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >  Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> >  
> >  Reviewed-by: Simon Horman <horms@kernel.org>
> >
>

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

[Dan Carpenter]

It's pretty common to have multiple Fixes tags.  But I don't think the
AI is correct in this case, actually.  I agree with you that it doesn't
really make sense to split the patch up into two half bugfixes just to
make backporting easier.  Plus the affected kernels would be six years
old by now.

regards,
dan carpenter

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

[Simon Horman]

On Wed, Feb 25, 2026 at 10:16:40AM +0000, Jiayuan Chen wrote:
> 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:
> > 
> > > 
> > > On Tue, Feb 24, 2026 at 12:46:38PM +0800, Jiayuan Chen wrote:
> > >  From: Jiayuan Chen <jiayuan.chen@shopee.com>
> > >  
> > >  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.

Thanks. I think it would be useful to include an explanation along
those lines in the commit message.

> 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.

Understood. I agree the code changes make sense to keep together
in a single patch. And retain the Fixes tag as you have it.

...

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

[Dan Carpenter]

On Tue, Feb 24, 2026 at 12:46:38PM +0800, Jiayuan Chen wrote:
> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> 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().
> 
> Reported-by: syzbot+72e3ea390c305de0e259@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@google.com/T/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> ---
>  net/atm/lec.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index afb8d3eb2185..a5b80d6df603 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -1260,24 +1260,27 @@ static void lec_arp_clear_vccs(struct lec_arp_table *entry)
>  		struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
>  		struct net_device *dev = (struct net_device *)vcc->proto_data;
>  
> -		vcc->pop = vpriv->old_pop;
> -		if (vpriv->xoff)
> -			netif_wake_queue(dev);
> -		kfree(vpriv);
> -		vcc->user_back = NULL;
> -		vcc->push = entry->old_push;
> -		vcc_release_async(vcc, -EPIPE);
> +		if (vpriv) {
> +			vcc->pop = vpriv->old_pop;
> +			if (vpriv->xoff)
> +				netif_wake_queue(dev);
> +			kfree(vpriv);
> +			vcc->user_back = NULL;
> +			vcc->push = entry->old_push;
> +			vcc_release_async(vcc, -EPIPE);
> +		}
>  		entry->vcc = NULL;
>  	}
>  	if (entry->recv_vcc) {
>  		struct atm_vcc *vcc = entry->recv_vcc;
>  		struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
>  
> -		kfree(vpriv);
> -		vcc->user_back = NULL;
> -
> -		entry->recv_vcc->push = entry->old_recv_push;
> -		vcc_release_async(entry->recv_vcc, -EPIPE);
> +		if (vpriv) {
> +			kfree(vpriv);
> +			vcc->user_back = NULL;
> +			vcc->push = entry->old_recv_push;
> +			vcc_release_async(vcc, -EPIPE);

I wasn't going say anything, but since it seems like maybe you're going
to redo this patch anyway.  Changing "entry->recv_vcc->push" to
"vcc->push" is obviously nice but could you do that in a separate
patch?

I use a tool to strip out the indenting changes:
https://github.com/error27/rename_rev

So when I review a patch like this, I want to pipe it to my script and
just see:

+		if (vpriv) {
			vcc->pop = vpriv->old_pop;
			if (vpriv->xoff)
				netif_wake_queue(dev);
			kfree(vpriv);
			vcc->user_back = NULL;
			vcc->push = entry->old_push;
			vcc_release_async(vcc, -EPIPE);
+		}
  		entry->vcc = NULL;
  	}
  	if (entry->recv_vcc) {
  		struct atm_vcc *vcc = entry->recv_vcc;
  		struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);

+		if (vpriv) {
			kfree(vpriv);
			vcc->user_back = NULL;

			entry->recv_vcc->push = entry->old_recv_push;
			vcc_release_async(entry->recv_vcc, -EPIPE);
+		}

The renames are nice but now I have to check things by hand.

regards,
dan carpenter

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

[Jiayuan Chen]

February 25, 2026 at 18:58, "Dan Carpenter" <dan.carpenter@linaro.org mailto:dan.carpenter@linaro.org?to=%22Dan%20Carpenter%22%20%3Cdan.carpenter%40linaro.org%3E > wrote:


> 
> On Tue, Feb 24, 2026 at 12:46:38PM +0800, Jiayuan Chen wrote:
> 
> > 
> > From: Jiayuan Chen <jiayuan.chen@shopee.com>
> >  
> >  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().
> >  
> >  Reported-by: syzbot+72e3ea390c305de0e259@syzkaller.appspotmail.com
> >  Closes: https://lore.kernel.org/all/68c95a83.050a0220.3c6139.0e5c.GAE@google.com/T/
> >  Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >  Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> >  ---
> >  net/atm/lec.c | 27 +++++++++++++++------------
> >  1 file changed, 15 insertions(+), 12 deletions(-)
> >  
> >  diff --git a/net/atm/lec.c b/net/atm/lec.c
> >  index afb8d3eb2185..a5b80d6df603 100644
> >  --- a/net/atm/lec.c
> >  +++ b/net/atm/lec.c
> >  @@ -1260,24 +1260,27 @@ static void lec_arp_clear_vccs(struct lec_arp_table *entry)
> >  struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
> >  struct net_device *dev = (struct net_device *)vcc->proto_data;
> >  
> >  - vcc->pop = vpriv->old_pop;
> >  - if (vpriv->xoff)
> >  - netif_wake_queue(dev);
> >  - kfree(vpriv);
> >  - vcc->user_back = NULL;
> >  - vcc->push = entry->old_push;
> >  - vcc_release_async(vcc, -EPIPE);
> >  + if (vpriv) {
> >  + vcc->pop = vpriv->old_pop;
> >  + if (vpriv->xoff)
> >  + netif_wake_queue(dev);
> >  + kfree(vpriv);
> >  + vcc->user_back = NULL;
> >  + vcc->push = entry->old_push;
> >  + vcc_release_async(vcc, -EPIPE);
> >  + }
> >  entry->vcc = NULL;
> >  }
> >  if (entry->recv_vcc) {
> >  struct atm_vcc *vcc = entry->recv_vcc;
> >  struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
> >  
> >  - kfree(vpriv);
> >  - vcc->user_back = NULL;
> >  -
> >  - entry->recv_vcc->push = entry->old_recv_push;
> >  - vcc_release_async(entry->recv_vcc, -EPIPE);
> >  + if (vpriv) {
> >  + kfree(vpriv);
> >  + vcc->user_back = NULL;
> >  + vcc->push = entry->old_recv_push;
> >  + vcc_release_async(vcc, -EPIPE);
> > 
> I wasn't going say anything, but since it seems like maybe you're going
> to redo this patch anyway. Changing "entry->recv_vcc->push" to
> "vcc->push" is obviously nice but could you do that in a separate
> patch?
> 
> I use a tool to strip out the indenting changes:
> https://github.com/error27/rename_rev
> 
> So when I review a patch like this, I want to pipe it to my script and
> just see:
> 
> + if (vpriv) {
>  vcc->pop = vpriv->old_pop;
>  if (vpriv->xoff)
>  netif_wake_queue(dev);
>  kfree(vpriv);
>  vcc->user_back = NULL;
>  vcc->push = entry->old_push;
>  vcc_release_async(vcc, -EPIPE);
> + }
>  entry->vcc = NULL;
>  }
>  if (entry->recv_vcc) {
>  struct atm_vcc *vcc = entry->recv_vcc;
>  struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
> 
> + if (vpriv) {
>  kfree(vpriv);
>  vcc->user_back = NULL;
> 
>  entry->recv_vcc->push = entry->old_recv_push;
>  vcc_release_async(entry->recv_vcc, -EPIPE);
> + }
> 
> The renames are nice but now I have to check things by hand.



Hi Dan,

Thanks for the review. You're right, the entry->recv_vcc->push to vcc->push rename should not be mixed into the bug fix patch.

I think I should only adds the if (vpriv) guards without any other changes.

I'll drop the cleanup rename entirely since this is legacy code and not worth a separate patch.

Best,
Jiayuan

> regards,
> dan carpenter
>