net/atm/lec.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)
A race condition exists between lec_atm_close() setting priv->lecd = NULL
and concurrent access to priv->lecd in send_to_lecd(), lec_handle_bridge(),
and lec_atm_send(). When the socket is freed via RCU while another thread
is still using it, a use-after-free occurs in sock_def_readable() when
accessing the socket's wait queue.
The root cause is that lec_atm_close() clears priv->lecd without holding
lec_arp_lock, while callers dereference priv->lecd without any protection
against concurrent teardown.
Fix this by:
- Protecting priv->lecd = NULL in lec_atm_close() with lec_arp_lock to
synchronize with callers that already hold the lock (e.g. lec_arp_resolve)
- Using sock_hold/sock_put in send_to_lecd() to pin the socket while in
use. This is safe because send_to_lecd() is called under lec_arp_lock
by lec_arp_resolve(), preventing concurrent NULL assignment of lecd.
- Using lec_arp_lock + sock_hold/sock_put in lec_handle_bridge() and
lec_atm_send() where the lock is not held by the caller, with proper
skb cleanup on early exit to avoid memory leaks.
Note: Patch testing via syzbot was attempted but the test VM crashed
due to a QEMU AHCI emulation assertion failure (hw/ide/core.c:934)
unrelated to this fix. The QEMU crash is caused by syzbot's disk I/O
fuzzing triggering a known QEMU NCQ emulation bug, not by this patch.
Compile testing with "make W=1 net/atm/lec.o" passes cleanly.
Reported-by: syzbot+f50072212ab792c86925@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f50072212ab792c86925
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
net/atm/lec.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/net/atm/lec.c b/net/atm/lec.c
index fb93c6e1c329..7e051174a92b 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -131,6 +131,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
{
char *buff;
struct lec_priv *priv;
+ unsigned long flags;
/*
* Check if this is a BPDU. If so, ask zeppelin to send
@@ -154,10 +155,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
/* 0x01 is topology change */
priv = netdev_priv(dev);
- atm_force_charge(priv->lecd, skb2->truesize);
+ spin_lock_irqsave(&priv->lec_arp_lock, flags);
+ if (!priv->lecd) {
+ spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
+ kfree_skb(skb2);
+ return;
+ }
sk = sk_atm(priv->lecd);
+ sock_hold(sk);
+ spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
+ atm_force_charge(priv->lecd, skb2->truesize);
skb_queue_tail(&sk->sk_receive_queue, skb2);
sk->sk_data_ready(sk);
+ sock_put(sk);
}
}
#endif /* IS_ENABLED(CONFIG_BRIDGE) */
@@ -441,7 +451,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
/* hit from bridge table, send LE_ARP_RESPONSE */
struct sk_buff *skb2;
struct sock *sk;
-
+ unsigned long flags;
pr_debug("%s: entry found, responding to zeppelin\n",
dev->name);
skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
@@ -449,10 +459,19 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
break;
skb2->len = sizeof(struct atmlec_msg);
skb_copy_to_linear_data(skb2, mesg, sizeof(*mesg));
- atm_force_charge(priv->lecd, skb2->truesize);
+ spin_lock_irqsave(&priv->lec_arp_lock, flags);
+ if (!priv->lecd) {
+ spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
+ kfree_skb(skb2);
+ break;
+ }
sk = sk_atm(priv->lecd);
+ sock_hold(sk);
+ spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
+ atm_force_charge(priv->lecd, skb2->truesize);
skb_queue_tail(&sk->sk_receive_queue, skb2);
sk->sk_data_ready(sk);
+ sock_put(sk);
}
}
#endif /* IS_ENABLED(CONFIG_BRIDGE) */
@@ -471,8 +490,11 @@ static void lec_atm_close(struct atm_vcc *vcc)
struct sk_buff *skb;
struct net_device *dev = (struct net_device *)vcc->proto_data;
struct lec_priv *priv = netdev_priv(dev);
+ unsigned long flags;
+ spin_lock_irqsave(&priv->lec_arp_lock, flags);
priv->lecd = NULL;
+ spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
/* Do something needful? */
netif_stop_queue(dev);
@@ -534,6 +556,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
atm_force_charge(priv->lecd, skb->truesize);
sk = sk_atm(priv->lecd);
+ sock_hold(sk);
skb_queue_tail(&sk->sk_receive_queue, skb);
sk->sk_data_ready(sk);
@@ -543,7 +566,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
skb_queue_tail(&sk->sk_receive_queue, data);
sk->sk_data_ready(sk);
}
-
+ sock_put(sk);
return 0;
}
--
2.43.0On Mon, Mar 9, 2026 at 10:36 AM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> A race condition exists between lec_atm_close() setting priv->lecd = NULL
> and concurrent access to priv->lecd in send_to_lecd(), lec_handle_bridge(),
> and lec_atm_send(). When the socket is freed via RCU while another thread
> is still using it, a use-after-free occurs in sock_def_readable() when
> accessing the socket's wait queue.
>
> The root cause is that lec_atm_close() clears priv->lecd without holding
> lec_arp_lock, while callers dereference priv->lecd without any protection
> against concurrent teardown.
>
> Fix this by:
> - Protecting priv->lecd = NULL in lec_atm_close() with lec_arp_lock to
> synchronize with callers that already hold the lock (e.g. lec_arp_resolve)
> - Using sock_hold/sock_put in send_to_lecd() to pin the socket while in
> use. This is safe because send_to_lecd() is called under lec_arp_lock
> by lec_arp_resolve(), preventing concurrent NULL assignment of lecd.
> - Using lec_arp_lock + sock_hold/sock_put in lec_handle_bridge() and
> lec_atm_send() where the lock is not held by the caller, with proper
> skb cleanup on early exit to avoid memory leaks.
>
> Note: Patch testing via syzbot was attempted but the test VM crashed
> due to a QEMU AHCI emulation assertion failure (hw/ide/core.c:934)
> unrelated to this fix. The QEMU crash is caused by syzbot's disk I/O
> fuzzing triggering a known QEMU NCQ emulation bug, not by this patch.
> Compile testing with "make W=1 net/atm/lec.o" passes cleanly.
>
> Reported-by: syzbot+f50072212ab792c86925@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f50072212ab792c86925
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> net/atm/lec.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index fb93c6e1c329..7e051174a92b 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -131,6 +131,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> {
> char *buff;
> struct lec_priv *priv;
> + unsigned long flags;
>
> /*
> * Check if this is a BPDU. If so, ask zeppelin to send
> @@ -154,10 +155,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> /* 0x01 is topology change */
>
> priv = netdev_priv(dev);
> - atm_force_charge(priv->lecd, skb2->truesize);
> + spin_lock_irqsave(&priv->lec_arp_lock, flags);
> + if (!priv->lecd) {
> + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> + kfree_skb(skb2);
> + return;
> + }
> sk = sk_atm(priv->lecd);
> + sock_hold(sk);
> + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
What prevents priv->lecd to be NULL after you released priv->lec_arp_lock ?
> + atm_force_charge(priv->lecd, skb2->truesize);
> skb_queue_tail(&sk->sk_receive_queue, skb2);
> sk->sk_data_ready(sk);
> + sock_put(sk);
> }
> }
> #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -441,7 +451,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
> /* hit from bridge table, send LE_ARP_RESPONSE */
> struct sk_buff *skb2;
> struct sock *sk;
> -
> + unsigned long flags;
> pr_debug("%s: entry found, responding to zeppelin\n",
> dev->name);
> skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
> @@ -449,10 +459,19 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
> break;
> skb2->len = sizeof(struct atmlec_msg);
> skb_copy_to_linear_data(skb2, mesg, sizeof(*mesg));
> - atm_force_charge(priv->lecd, skb2->truesize);
> + spin_lock_irqsave(&priv->lec_arp_lock, flags);
> + if (!priv->lecd) {
> + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> + kfree_skb(skb2);
> + break;
> + }
> sk = sk_atm(priv->lecd);
> + sock_hold(sk);
> + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> + atm_force_charge(priv->lecd, skb2->truesize);
> skb_queue_tail(&sk->sk_receive_queue, skb2);
> sk->sk_data_ready(sk);
> + sock_put(sk);
> }
> }
> #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -471,8 +490,11 @@ static void lec_atm_close(struct atm_vcc *vcc)
> struct sk_buff *skb;
> struct net_device *dev = (struct net_device *)vcc->proto_data;
> struct lec_priv *priv = netdev_priv(dev);
> + unsigned long flags;
>
> + spin_lock_irqsave(&priv->lec_arp_lock, flags);
> priv->lecd = NULL;
> + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> /* Do something needful? */
>
> netif_stop_queue(dev);
> @@ -534,6 +556,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
>
> atm_force_charge(priv->lecd, skb->truesize);
> sk = sk_atm(priv->lecd);
> + sock_hold(sk);
> skb_queue_tail(&sk->sk_receive_queue, skb);
> sk->sk_data_ready(sk);
>
> @@ -543,7 +566,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
> skb_queue_tail(&sk->sk_receive_queue, data);
> sk->sk_data_ready(sk);
> }
> -
> + sock_put(sk);
> return 0;
> }
>
> --
> 2.43.0
>
On Mon, Mar 9, 2026 at 10:55 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Mar 9, 2026 at 10:36 AM Deepanshu Kartikey
> <kartikey406@gmail.com> wrote:
> >
> > A race condition exists between lec_atm_close() setting priv->lecd = NULL
> > and concurrent access to priv->lecd in send_to_lecd(), lec_handle_bridge(),
> > and lec_atm_send(). When the socket is freed via RCU while another thread
> > is still using it, a use-after-free occurs in sock_def_readable() when
> > accessing the socket's wait queue.
> >
> > The root cause is that lec_atm_close() clears priv->lecd without holding
> > lec_arp_lock, while callers dereference priv->lecd without any protection
> > against concurrent teardown.
> >
> > Fix this by:
> > - Protecting priv->lecd = NULL in lec_atm_close() with lec_arp_lock to
> > synchronize with callers that already hold the lock (e.g. lec_arp_resolve)
> > - Using sock_hold/sock_put in send_to_lecd() to pin the socket while in
> > use. This is safe because send_to_lecd() is called under lec_arp_lock
> > by lec_arp_resolve(), preventing concurrent NULL assignment of lecd.
> > - Using lec_arp_lock + sock_hold/sock_put in lec_handle_bridge() and
> > lec_atm_send() where the lock is not held by the caller, with proper
> > skb cleanup on early exit to avoid memory leaks.
> >
> > Note: Patch testing via syzbot was attempted but the test VM crashed
> > due to a QEMU AHCI emulation assertion failure (hw/ide/core.c:934)
> > unrelated to this fix. The QEMU crash is caused by syzbot's disk I/O
> > fuzzing triggering a known QEMU NCQ emulation bug, not by this patch.
> > Compile testing with "make W=1 net/atm/lec.o" passes cleanly.
> >
> > Reported-by: syzbot+f50072212ab792c86925@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f50072212ab792c86925
> > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > ---
> > net/atm/lec.c | 31 +++++++++++++++++++++++++++----
> > 1 file changed, 27 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/atm/lec.c b/net/atm/lec.c
> > index fb93c6e1c329..7e051174a92b 100644
> > --- a/net/atm/lec.c
> > +++ b/net/atm/lec.c
> > @@ -131,6 +131,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> > {
> > char *buff;
> > struct lec_priv *priv;
> > + unsigned long flags;
> >
> > /*
> > * Check if this is a BPDU. If so, ask zeppelin to send
> > @@ -154,10 +155,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> > /* 0x01 is topology change */
> >
> > priv = netdev_priv(dev);
> > - atm_force_charge(priv->lecd, skb2->truesize);
> > + spin_lock_irqsave(&priv->lec_arp_lock, flags);
> > + if (!priv->lecd) {
> > + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> > + kfree_skb(skb2);
> > + return;
> > + }
> > sk = sk_atm(priv->lecd);
> > + sock_hold(sk);
> > + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
>
> What prevents priv->lecd to be NULL after you released priv->lec_arp_lock ?
More generally, lec_atm_close() clears the sk_receive_queue.
So allowing providers to queue more packets would be wrong.
So really a better fix is needed.
On Mon, Mar 9, 2026 at 3:29 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Mar 9, 2026 at 10:55 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Mon, Mar 9, 2026 at 10:36 AM Deepanshu Kartikey
> > <kartikey406@gmail.com> wrote:
> > >
> > > A race condition exists between lec_atm_close() setting priv->lecd = NULL
> > > and concurrent access to priv->lecd in send_to_lecd(), lec_handle_bridge(),
> > > and lec_atm_send(). When the socket is freed via RCU while another thread
> > > is still using it, a use-after-free occurs in sock_def_readable() when
> > > accessing the socket's wait queue.
> > >
> > > The root cause is that lec_atm_close() clears priv->lecd without holding
> > > lec_arp_lock, while callers dereference priv->lecd without any protection
> > > against concurrent teardown.
> > >
> > > Fix this by:
> > > - Protecting priv->lecd = NULL in lec_atm_close() with lec_arp_lock to
> > > synchronize with callers that already hold the lock (e.g. lec_arp_resolve)
> > > - Using sock_hold/sock_put in send_to_lecd() to pin the socket while in
> > > use. This is safe because send_to_lecd() is called under lec_arp_lock
> > > by lec_arp_resolve(), preventing concurrent NULL assignment of lecd.
> > > - Using lec_arp_lock + sock_hold/sock_put in lec_handle_bridge() and
> > > lec_atm_send() where the lock is not held by the caller, with proper
> > > skb cleanup on early exit to avoid memory leaks.
> > >
> > > Note: Patch testing via syzbot was attempted but the test VM crashed
> > > due to a QEMU AHCI emulation assertion failure (hw/ide/core.c:934)
> > > unrelated to this fix. The QEMU crash is caused by syzbot's disk I/O
> > > fuzzing triggering a known QEMU NCQ emulation bug, not by this patch.
> > > Compile testing with "make W=1 net/atm/lec.o" passes cleanly.
> > >
> > > Reported-by: syzbot+f50072212ab792c86925@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=f50072212ab792c86925
> > > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > > ---
> > > net/atm/lec.c | 31 +++++++++++++++++++++++++++----
> > > 1 file changed, 27 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/net/atm/lec.c b/net/atm/lec.c
> > > index fb93c6e1c329..7e051174a92b 100644
> > > --- a/net/atm/lec.c
> > > +++ b/net/atm/lec.c
> > > @@ -131,6 +131,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> > > {
> > > char *buff;
> > > struct lec_priv *priv;
> > > + unsigned long flags;
> > >
> > > /*
> > > * Check if this is a BPDU. If so, ask zeppelin to send
> > > @@ -154,10 +155,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> > > /* 0x01 is topology change */
> > >
> > > priv = netdev_priv(dev);
> > > - atm_force_charge(priv->lecd, skb2->truesize);
> > > + spin_lock_irqsave(&priv->lec_arp_lock, flags);
> > > + if (!priv->lecd) {
> > > + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> > > + kfree_skb(skb2);
> > > + return;
> > > + }
> > > sk = sk_atm(priv->lecd);
> > > + sock_hold(sk);
> > > + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> >
> > What prevents priv->lecd to be NULL after you released priv->lec_arp_lock ?
>
> More generally, lec_atm_close() clears the sk_receive_queue.
>
> So allowing providers to queue more packets would be wrong.
>
> So really a better fix is needed.
Thank you for the review Eric.
You are correct on both points.
1. After releasing lec_arp_lock, priv->lecd can be set to NULL
by lec_atm_close() concurrently. I should have saved a local
copy of priv->lecd before releasing the lock.
2. The deeper issue is that lec_atm_close() drains sk_receive_queue
but my spinlock approach does not prevent new packets from being
queued after the drain, since timer and workqueue paths like
lec_arp_work bypass netif_stop_queue().
I think the correct fix would be to use RCU:
- Mark priv->lecd as __rcu in lec.h
- Use rcu_assign_pointer() in lec_atm_close() and lecd_attach()
- Use rcu_read_lock/rcu_dereference in all send paths
- Use synchronize_rcu() in lec_atm_close() before draining
the queue, ensuring all senders have completed before drain
This way synchronize_rcu() guarantees no new packets can be
queued after the drain begins.
Does this approach look correct to you?
Deepanshu Kartikey
On Mon, Mar 9, 2026 at 11:34 AM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> On Mon, Mar 9, 2026 at 3:29 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Mon, Mar 9, 2026 at 10:55 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Mon, Mar 9, 2026 at 10:36 AM Deepanshu Kartikey
> > > <kartikey406@gmail.com> wrote:
> > > >
> > > > A race condition exists between lec_atm_close() setting priv->lecd = NULL
> > > > and concurrent access to priv->lecd in send_to_lecd(), lec_handle_bridge(),
> > > > and lec_atm_send(). When the socket is freed via RCU while another thread
> > > > is still using it, a use-after-free occurs in sock_def_readable() when
> > > > accessing the socket's wait queue.
> > > >
> > > > The root cause is that lec_atm_close() clears priv->lecd without holding
> > > > lec_arp_lock, while callers dereference priv->lecd without any protection
> > > > against concurrent teardown.
> > > >
> > > > Fix this by:
> > > > - Protecting priv->lecd = NULL in lec_atm_close() with lec_arp_lock to
> > > > synchronize with callers that already hold the lock (e.g. lec_arp_resolve)
> > > > - Using sock_hold/sock_put in send_to_lecd() to pin the socket while in
> > > > use. This is safe because send_to_lecd() is called under lec_arp_lock
> > > > by lec_arp_resolve(), preventing concurrent NULL assignment of lecd.
> > > > - Using lec_arp_lock + sock_hold/sock_put in lec_handle_bridge() and
> > > > lec_atm_send() where the lock is not held by the caller, with proper
> > > > skb cleanup on early exit to avoid memory leaks.
> > > >
> > > > Note: Patch testing via syzbot was attempted but the test VM crashed
> > > > due to a QEMU AHCI emulation assertion failure (hw/ide/core.c:934)
> > > > unrelated to this fix. The QEMU crash is caused by syzbot's disk I/O
> > > > fuzzing triggering a known QEMU NCQ emulation bug, not by this patch.
> > > > Compile testing with "make W=1 net/atm/lec.o" passes cleanly.
> > > >
> > > > Reported-by: syzbot+f50072212ab792c86925@syzkaller.appspotmail.com
> > > > Closes: https://syzkaller.appspot.com/bug?extid=f50072212ab792c86925
> > > > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > > > ---
> > > > net/atm/lec.c | 31 +++++++++++++++++++++++++++----
> > > > 1 file changed, 27 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/net/atm/lec.c b/net/atm/lec.c
> > > > index fb93c6e1c329..7e051174a92b 100644
> > > > --- a/net/atm/lec.c
> > > > +++ b/net/atm/lec.c
> > > > @@ -131,6 +131,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> > > > {
> > > > char *buff;
> > > > struct lec_priv *priv;
> > > > + unsigned long flags;
> > > >
> > > > /*
> > > > * Check if this is a BPDU. If so, ask zeppelin to send
> > > > @@ -154,10 +155,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> > > > /* 0x01 is topology change */
> > > >
> > > > priv = netdev_priv(dev);
> > > > - atm_force_charge(priv->lecd, skb2->truesize);
> > > > + spin_lock_irqsave(&priv->lec_arp_lock, flags);
> > > > + if (!priv->lecd) {
> > > > + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> > > > + kfree_skb(skb2);
> > > > + return;
> > > > + }
> > > > sk = sk_atm(priv->lecd);
> > > > + sock_hold(sk);
> > > > + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> > >
> > > What prevents priv->lecd to be NULL after you released priv->lec_arp_lock ?
> >
> > More generally, lec_atm_close() clears the sk_receive_queue.
> >
> > So allowing providers to queue more packets would be wrong.
> >
> > So really a better fix is needed.
>
> Thank you for the review Eric.
>
> You are correct on both points.
>
> 1. After releasing lec_arp_lock, priv->lecd can be set to NULL
> by lec_atm_close() concurrently. I should have saved a local
> copy of priv->lecd before releasing the lock.
>
> 2. The deeper issue is that lec_atm_close() drains sk_receive_queue
> but my spinlock approach does not prevent new packets from being
> queued after the drain, since timer and workqueue paths like
> lec_arp_work bypass netif_stop_queue().
>
> I think the correct fix would be to use RCU:
> - Mark priv->lecd as __rcu in lec.h
> - Use rcu_assign_pointer() in lec_atm_close() and lecd_attach()
> - Use rcu_read_lock/rcu_dereference in all send paths
> - Use synchronize_rcu() in lec_atm_close() before draining
> the queue, ensuring all senders have completed before drain
>
> This way synchronize_rcu() guarantees no new packets can be
> queued after the drain begins.
>
> Does this approach look correct to you?
This would be good, make sure lec_atm_close() is allowed to sleep.
If this is vcc_destroy_socket(), maybe sk_receive_queue purging
already happens there.
On Mon, Mar 9, 2026 at 4:32 PM Eric Dumazet <edumazet@google.com> wrote: > > This would be good, make sure lec_atm_close() is allowed to sleep. > > If this is vcc_destroy_socket(), maybe sk_receive_queue purging > already happens there. Thank you for the suggestion Eric. I will send a v2 patch based on these findings. Deepanshu Kartikey
On Mon, Mar 09, 2026 at 03:06:14PM +0800, Deepanshu Kartikey wrote:
> A race condition exists between lec_atm_close() setting priv->lecd = NULL
> and concurrent access to priv->lecd in send_to_lecd(), lec_handle_bridge(),
> and lec_atm_send(). When the socket is freed via RCU while another thread
> is still using it, a use-after-free occurs in sock_def_readable() when
> accessing the socket's wait queue.
>
> The root cause is that lec_atm_close() clears priv->lecd without holding
> lec_arp_lock, while callers dereference priv->lecd without any protection
> against concurrent teardown.
>
> Fix this by:
> - Protecting priv->lecd = NULL in lec_atm_close() with lec_arp_lock to
> synchronize with callers that already hold the lock (e.g. lec_arp_resolve)
> - Using sock_hold/sock_put in send_to_lecd() to pin the socket while in
> use. This is safe because send_to_lecd() is called under lec_arp_lock
> by lec_arp_resolve(), preventing concurrent NULL assignment of lecd.
> - Using lec_arp_lock + sock_hold/sock_put in lec_handle_bridge() and
> lec_atm_send() where the lock is not held by the caller, with proper
> skb cleanup on early exit to avoid memory leaks.
This is wrong. send_to_lecd() is also called from lec_arp_expire_arp() without lec_arp_lock.
lec_arp_expire_arp() // no lec_arp_lock held
→ send_to_lecd()
→ sk = sk_atm(priv->lecd) // UAF: lecd already freed
→ sock_hold(sk) // too late
The sock_hold() is placed after the dereference of priv->lecd,
so it cannot prevent the UAF — the dangling pointer has already been followed.
> Note: Patch testing via syzbot was attempted but the test VM crashed
> due to a QEMU AHCI emulation assertion failure (hw/ide/core.c:934)
> unrelated to this fix. The QEMU crash is caused by syzbot's disk I/O
> fuzzing triggering a known QEMU NCQ emulation bug, not by this patch.
> Compile testing with "make W=1 net/atm/lec.o" passes cleanly.
>
> Reported-by: syzbot+f50072212ab792c86925@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f50072212ab792c86925
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> net/atm/lec.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index fb93c6e1c329..7e051174a92b 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -131,6 +131,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> {
> char *buff;
> struct lec_priv *priv;
> + unsigned long flags;
>
> /*
> * Check if this is a BPDU. If so, ask zeppelin to send
> @@ -154,10 +155,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
> /* 0x01 is topology change */
>
> priv = netdev_priv(dev);
> - atm_force_charge(priv->lecd, skb2->truesize);
> + spin_lock_irqsave(&priv->lec_arp_lock, flags);
> + if (!priv->lecd) {
> + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> + kfree_skb(skb2);
> + return;
> + }
> sk = sk_atm(priv->lecd);
> + sock_hold(sk);
> + spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
> + atm_force_charge(priv->lecd, skb2->truesize);
After the unlock, lec_atm_close() can set priv->lecd = NULL
© 2016 - 2026 Red Hat, Inc.