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

[Simon Horman <horms@kernel.org>]

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