* [PATCH v2] atm: lec: fix use-after-free in sock_def_readable()
@ 2026-03-09 15:59 Deepanshu Kartikey
2026-03-10 3:18 ` Hillf Danton
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Deepanshu Kartikey @ 2026-03-09 15:59 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, horms
Cc: mingo, tglx, jiayuan.chen, kees, netdev, dan.carpenter,
linux-kernel, Deepanshu Kartikey, syzbot+f50072212ab792c86925
A race condition exists between lec_atm_close() setting priv->lecd
to 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
any synchronization, while callers dereference priv->lecd without
any protection against concurrent teardown.
Fix this by converting priv->lecd to an RCU-protected pointer:
- Mark priv->lecd as __rcu in lec.h
- Use rcu_assign_pointer() in lec_atm_close() and lecd_attach()
for safe pointer assignment
- Use rcu_access_pointer() for NULL checks that do not dereference
the pointer in lec_start_xmit(), lec_push(), send_to_lecd() and
lecd_attach()
- Use rcu_read_lock/rcu_dereference/rcu_read_unlock in send_to_lecd(),
lec_handle_bridge() and lec_atm_send() to safely access lecd
- Use rcu_assign_pointer() followed by synchronize_rcu() in
lec_atm_close() to ensure all readers have completed before
proceeding. This is safe since lec_atm_close() is called from
vcc_release() which holds lock_sock(), a sleeping lock.
- Remove the manual sk_receive_queue drain from lec_atm_close()
since vcc_destroy_socket() already drains it after lec_atm_close()
returns.
v2: Switch from spinlock + sock_hold/put approach to RCU to properly
fix the race. The v1 spinlock approach had two issues pointed out
by Eric Dumazet:
1. priv->lecd was still accessed directly after releasing the
lock instead of using a local copy.
2. The spinlock did not prevent packets being queued after
lec_atm_close() drains sk_receive_queue since timer and
workqueue paths bypass netif_stop_queue().
Note: Syzbot patch testing was attempted but the test VM terminated
unexpectedly with "Connection to localhost closed by remote host",
likely due to a QEMU AHCI emulation issue unrelated to this fix.
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
Link: https://lore.kernel.org/all/20260309093614.502094-1-kartikey406@gmail.com/T/ [v1]
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
net/atm/lec.c | 70 +++++++++++++++++++++++++++++++++------------------
net/atm/lec.h | 2 +-
2 files changed, 46 insertions(+), 26 deletions(-)
diff --git a/net/atm/lec.c b/net/atm/lec.c
index fb93c6e1c329..32adf28663a0 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -154,10 +154,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);
- sk = sk_atm(priv->lecd);
- skb_queue_tail(&sk->sk_receive_queue, skb2);
- sk->sk_data_ready(sk);
+ struct atm_vcc *vcc;
+
+ rcu_read_lock();
+ vcc = rcu_dereference(priv->lecd);
+ if (vcc) {
+ atm_force_charge(vcc, skb2->truesize);
+ sk = sk_atm(vcc);
+ skb_queue_tail(&sk->sk_receive_queue, skb2);
+ sk->sk_data_ready(sk);
+ } else {
+ dev_kfree_skb(skb2);
+ }
+ rcu_read_unlock();
}
}
#endif /* IS_ENABLED(CONFIG_BRIDGE) */
@@ -216,7 +225,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
int is_rdesc;
pr_debug("called\n");
- if (!priv->lecd) {
+ if (!rcu_access_pointer(priv->lecd)) {
pr_info("%s:No lecd attached\n", dev->name);
dev->stats.tx_errors++;
netif_stop_queue(dev);
@@ -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);
- sk = sk_atm(priv->lecd);
- skb_queue_tail(&sk->sk_receive_queue, skb2);
- sk->sk_data_ready(sk);
+ struct atm_vcc *vcc;
+
+ rcu_read_lock();
+ vcc = rcu_dereference(priv->lecd);
+ if (vcc) {
+ atm_force_charge(vcc, skb2->truesize);
+ sk = sk_atm(vcc);
+ skb_queue_tail(&sk->sk_receive_queue, skb2);
+ sk->sk_data_ready(sk);
+ } else {
+ dev_kfree_skb(skb2);
+ }
+ rcu_read_unlock();
}
}
#endif /* IS_ENABLED(CONFIG_BRIDGE) */
@@ -468,23 +486,16 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
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);
- priv->lecd = NULL;
+ rcu_assign_pointer(priv->lecd, NULL);
+ synchronize_rcu();
/* Do something needful? */
netif_stop_queue(dev);
lec_arp_destroy(priv);
- if (skb_peek(&sk_atm(vcc)->sk_receive_queue))
- pr_info("%s closing with messages pending\n", dev->name);
- while ((skb = skb_dequeue(&sk_atm(vcc)->sk_receive_queue))) {
- atm_return(vcc, skb->truesize);
- dev_kfree_skb(skb);
- }
-
pr_info("%s: Shut down!\n", dev->name);
module_put(THIS_MODULE);
}
@@ -510,12 +521,14 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
const unsigned char *mac_addr, const unsigned char *atm_addr,
struct sk_buff *data)
{
+ struct atm_vcc *vcc;
struct sock *sk;
struct sk_buff *skb;
struct atmlec_msg *mesg;
- if (!priv || !priv->lecd)
+ if (!priv || !rcu_access_pointer(priv->lecd))
return -1;
+
skb = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
if (!skb)
return -1;
@@ -532,18 +545,27 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
if (atm_addr)
memcpy(&mesg->content.normal.atm_addr, atm_addr, ATM_ESA_LEN);
- atm_force_charge(priv->lecd, skb->truesize);
- sk = sk_atm(priv->lecd);
+ rcu_read_lock();
+ vcc = rcu_dereference(priv->lecd);
+ if (!vcc) {
+ rcu_read_unlock();
+ kfree_skb(skb);
+ return -1;
+ }
+
+ atm_force_charge(vcc, skb->truesize);
+ sk = sk_atm(vcc);
skb_queue_tail(&sk->sk_receive_queue, skb);
sk->sk_data_ready(sk);
if (data != NULL) {
pr_debug("about to send %d bytes of data\n", data->len);
- atm_force_charge(priv->lecd, data->truesize);
+ atm_force_charge(vcc, data->truesize);
skb_queue_tail(&sk->sk_receive_queue, data);
sk->sk_data_ready(sk);
}
+ rcu_read_unlock();
return 0;
}
@@ -618,7 +640,7 @@ static void lec_push(struct atm_vcc *vcc, struct sk_buff *skb)
atm_return(vcc, skb->truesize);
if (*(__be16 *) skb->data == htons(priv->lecid) ||
- !priv->lecd || !(dev->flags & IFF_UP)) {
+ !rcu_access_pointer(priv->lecd) || !(dev->flags & IFF_UP)) {
/*
* Probably looping back, or if lecd is missing,
* lecd has gone down
@@ -753,12 +775,12 @@ static int lecd_attach(struct atm_vcc *vcc, int arg)
priv = netdev_priv(dev_lec[i]);
} else {
priv = netdev_priv(dev_lec[i]);
- if (priv->lecd)
+ if (rcu_access_pointer(priv->lecd))
return -EADDRINUSE;
}
lec_arp_init(priv);
priv->itfnum = i; /* LANE2 addition */
- priv->lecd = vcc;
+ rcu_assign_pointer(priv->lecd, vcc);
vcc->dev = &lecatm_dev;
vcc_insert_socket(sk_atm(vcc));
diff --git a/net/atm/lec.h b/net/atm/lec.h
index be0e2667bd8c..ec85709bf818 100644
--- a/net/atm/lec.h
+++ b/net/atm/lec.h
@@ -91,7 +91,7 @@ struct lec_priv {
*/
spinlock_t lec_arp_lock;
struct atm_vcc *mcast_vcc; /* Default Multicast Send VCC */
- struct atm_vcc *lecd;
+ struct atm_vcc __rcu *lecd;
struct delayed_work lec_arp_work; /* C10 */
unsigned int maximum_unknown_frame_count;
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] atm: lec: fix use-after-free in sock_def_readable()
2026-03-09 15:59 [PATCH v2] atm: lec: fix use-after-free in sock_def_readable() Deepanshu Kartikey
@ 2026-03-10 3:18 ` Hillf Danton
2026-03-12 1:03 ` Deepanshu Kartikey
2026-03-14 12:00 ` Deepanshu Kartikey
2026-03-14 15:40 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 7+ messages in thread
From: Hillf Danton @ 2026-03-10 3:18 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: edumazet, netdev, linux-kernel, syzbot+f50072212ab792c86925
On Mon, 9 Mar 2026 21:29:08 +0530 Deepanshu Kartikey wrote:
> A race condition exists between lec_atm_close() setting priv->lecd
> to 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
> any synchronization, while callers dereference priv->lecd without
> any protection against concurrent teardown.
>
> Fix this by converting priv->lecd to an RCU-protected pointer:
> - Mark priv->lecd as __rcu in lec.h
> - Use rcu_assign_pointer() in lec_atm_close() and lecd_attach()
> for safe pointer assignment
> - Use rcu_access_pointer() for NULL checks that do not dereference
> the pointer in lec_start_xmit(), lec_push(), send_to_lecd() and
> lecd_attach()
> - Use rcu_read_lock/rcu_dereference/rcu_read_unlock in send_to_lecd(),
> lec_handle_bridge() and lec_atm_send() to safely access lecd
> - Use rcu_assign_pointer() followed by synchronize_rcu() in
> lec_atm_close() to ensure all readers have completed before
> proceeding. This is safe since lec_atm_close() is called from
> vcc_release() which holds lock_sock(), a sleeping lock.
> - Remove the manual sk_receive_queue drain from lec_atm_close()
> since vcc_destroy_socket() already drains it after lec_atm_close()
> returns.
>
> v2: Switch from spinlock + sock_hold/put approach to RCU to properly
> fix the race. The v1 spinlock approach had two issues pointed out
> by Eric Dumazet:
> 1. priv->lecd was still accessed directly after releasing the
> lock instead of using a local copy.
> 2. The spinlock did not prevent packets being queued after
> lec_atm_close() drains sk_receive_queue since timer and
> workqueue paths bypass netif_stop_queue().
>
> Note: Syzbot patch testing was attempted but the test VM terminated
> unexpectedly with "Connection to localhost closed by remote host",
> likely due to a QEMU AHCI emulation issue unrelated to this fix.
> 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
> Link: https://lore.kernel.org/all/20260309093614.502094-1-kartikey406@gmail.com/T/ [v1]
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> net/atm/lec.c | 70 +++++++++++++++++++++++++++++++++------------------
> net/atm/lec.h | 2 +-
> 2 files changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index fb93c6e1c329..32adf28663a0 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -154,10 +154,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);
> - sk = sk_atm(priv->lecd);
> - skb_queue_tail(&sk->sk_receive_queue, skb2);
> - sk->sk_data_ready(sk);
> + struct atm_vcc *vcc;
> +
> + rcu_read_lock();
> + vcc = rcu_dereference(priv->lecd);
> + if (vcc) {
> + atm_force_charge(vcc, skb2->truesize);
> + sk = sk_atm(vcc);
> + skb_queue_tail(&sk->sk_receive_queue, skb2);
> + sk->sk_data_ready(sk);
> + } else {
> + dev_kfree_skb(skb2);
> + }
> + rcu_read_unlock();
> }
> }
> #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -216,7 +225,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
> int is_rdesc;
>
> pr_debug("called\n");
> - if (!priv->lecd) {
> + if (!rcu_access_pointer(priv->lecd)) {
> pr_info("%s:No lecd attached\n", dev->name);
> dev->stats.tx_errors++;
> netif_stop_queue(dev);
> @@ -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);
> - sk = sk_atm(priv->lecd);
> - skb_queue_tail(&sk->sk_receive_queue, skb2);
> - sk->sk_data_ready(sk);
> + struct atm_vcc *vcc;
> +
> + rcu_read_lock();
> + vcc = rcu_dereference(priv->lecd);
> + if (vcc) {
> + atm_force_charge(vcc, skb2->truesize);
> + sk = sk_atm(vcc);
> + skb_queue_tail(&sk->sk_receive_queue, skb2);
> + sk->sk_data_ready(sk);
> + } else {
> + dev_kfree_skb(skb2);
> + }
> + rcu_read_unlock();
> }
> }
> #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -468,23 +486,16 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
>
> 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);
>
> - priv->lecd = NULL;
> + rcu_assign_pointer(priv->lecd, NULL);
> + synchronize_rcu();
> /* Do something needful? */
>
At this point priv->lecd is no longer used, so why not make lecd valid
throughout the lifespan of priv and free it after stopping dev queue,
instead of the tedious rcu trick?
> netif_stop_queue(dev);
> lec_arp_destroy(priv);
>
> - if (skb_peek(&sk_atm(vcc)->sk_receive_queue))
> - pr_info("%s closing with messages pending\n", dev->name);
> - while ((skb = skb_dequeue(&sk_atm(vcc)->sk_receive_queue))) {
> - atm_return(vcc, skb->truesize);
> - dev_kfree_skb(skb);
> - }
> -
> pr_info("%s: Shut down!\n", dev->name);
> module_put(THIS_MODULE);
> }
> @@ -510,12 +521,14 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
> const unsigned char *mac_addr, const unsigned char *atm_addr,
> struct sk_buff *data)
> {
> + struct atm_vcc *vcc;
> struct sock *sk;
> struct sk_buff *skb;
> struct atmlec_msg *mesg;
>
> - if (!priv || !priv->lecd)
> + if (!priv || !rcu_access_pointer(priv->lecd))
> return -1;
> +
> skb = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
> if (!skb)
> return -1;
> @@ -532,18 +545,27 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
> if (atm_addr)
> memcpy(&mesg->content.normal.atm_addr, atm_addr, ATM_ESA_LEN);
>
> - atm_force_charge(priv->lecd, skb->truesize);
> - sk = sk_atm(priv->lecd);
> + rcu_read_lock();
> + vcc = rcu_dereference(priv->lecd);
> + if (!vcc) {
> + rcu_read_unlock();
> + kfree_skb(skb);
> + return -1;
> + }
> +
> + atm_force_charge(vcc, skb->truesize);
> + sk = sk_atm(vcc);
> skb_queue_tail(&sk->sk_receive_queue, skb);
> sk->sk_data_ready(sk);
>
> if (data != NULL) {
> pr_debug("about to send %d bytes of data\n", data->len);
> - atm_force_charge(priv->lecd, data->truesize);
> + atm_force_charge(vcc, data->truesize);
> skb_queue_tail(&sk->sk_receive_queue, data);
> sk->sk_data_ready(sk);
> }
>
> + rcu_read_unlock();
> return 0;
> }
>
> @@ -618,7 +640,7 @@ static void lec_push(struct atm_vcc *vcc, struct sk_buff *skb)
>
> atm_return(vcc, skb->truesize);
> if (*(__be16 *) skb->data == htons(priv->lecid) ||
> - !priv->lecd || !(dev->flags & IFF_UP)) {
> + !rcu_access_pointer(priv->lecd) || !(dev->flags & IFF_UP)) {
> /*
> * Probably looping back, or if lecd is missing,
> * lecd has gone down
> @@ -753,12 +775,12 @@ static int lecd_attach(struct atm_vcc *vcc, int arg)
> priv = netdev_priv(dev_lec[i]);
> } else {
> priv = netdev_priv(dev_lec[i]);
> - if (priv->lecd)
> + if (rcu_access_pointer(priv->lecd))
> return -EADDRINUSE;
> }
> lec_arp_init(priv);
> priv->itfnum = i; /* LANE2 addition */
> - priv->lecd = vcc;
> + rcu_assign_pointer(priv->lecd, vcc);
> vcc->dev = &lecatm_dev;
> vcc_insert_socket(sk_atm(vcc));
>
> diff --git a/net/atm/lec.h b/net/atm/lec.h
> index be0e2667bd8c..ec85709bf818 100644
> --- a/net/atm/lec.h
> +++ b/net/atm/lec.h
> @@ -91,7 +91,7 @@ struct lec_priv {
> */
> spinlock_t lec_arp_lock;
> struct atm_vcc *mcast_vcc; /* Default Multicast Send VCC */
> - struct atm_vcc *lecd;
> + struct atm_vcc __rcu *lecd;
> struct delayed_work lec_arp_work; /* C10 */
> unsigned int maximum_unknown_frame_count;
> /*
> --
> 2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] atm: lec: fix use-after-free in sock_def_readable()
2026-03-10 3:18 ` Hillf Danton
@ 2026-03-12 1:03 ` Deepanshu Kartikey
2026-03-14 2:53 ` Hillf Danton
0 siblings, 1 reply; 7+ messages in thread
From: Deepanshu Kartikey @ 2026-03-12 1:03 UTC (permalink / raw)
To: Hillf Danton; +Cc: edumazet, netdev, linux-kernel, syzbot+f50072212ab792c86925
On Tue, Mar 10, 2026 at 8:48 AM Hillf Danton <hdanton@sina.com> wrote:
>
> At this point priv->lecd is no longer used, so why not make lecd valid
> throughout the lifespan of priv and free it after stopping dev queue,
> instead of the tedious rcu trick?
>
Thank you for the suggestion.
I investigated this approach. While netif_stop_queue() stops
the TX path and cancel_delayed_work_sync() in lec_arp_destroy()
stops lec_arp_work, the bug is actually triggered from
mld_ifc_work (IPv6 multicast workqueue) which calls:
mld_ifc_work -> mld_sendpack -> ip6_output
-> lec_start_xmit -> lec_arp_resolve -> send_to_lecd
This workqueue belongs to the IPv6 multicast subsystem and
is completely outside ATM/LEC control. Neither
netif_stop_queue() nor lec_arp_destroy() can stop it, so
simply reordering the calls in lec_atm_close() would not
fix the race.
The RCU approach with synchronize_rcu() ensures ALL callers
including mld_ifc_work have finished before priv->lecd is
cleared.
Deepanshu Kartikey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] atm: lec: fix use-after-free in sock_def_readable()
2026-03-12 1:03 ` Deepanshu Kartikey
@ 2026-03-14 2:53 ` Hillf Danton
0 siblings, 0 replies; 7+ messages in thread
From: Hillf Danton @ 2026-03-14 2:53 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: edumazet, netdev, linux-kernel, syzbot+f50072212ab792c86925
On Thu, 12 Mar 2026 06:33:53 +0530 Deepanshu Kartikey wrote:
> On Tue, Mar 10, 2026 at 8:48 AM Hillf Danton <hdanton@sina.com> wrote:
> >
> > At this point priv->lecd is no longer used, so why not make lecd valid
> > throughout the lifespan of priv and free it after stopping dev queue,
> > instead of the tedious rcu trick?
> >
>
> Thank you for the suggestion.
>
> I investigated this approach. While netif_stop_queue() stops
> the TX path and cancel_delayed_work_sync() in lec_arp_destroy()
> stops lec_arp_work, the bug is actually triggered from
> mld_ifc_work (IPv6 multicast workqueue) which calls:
>
> mld_ifc_work -> mld_sendpack -> ip6_output
> -> lec_start_xmit -> lec_arp_resolve -> send_to_lecd
>
> This workqueue belongs to the IPv6 multicast subsystem and
> is completely outside ATM/LEC control. Neither
> netif_stop_queue() nor lec_arp_destroy() can stop it, so
> simply reordering the calls in lec_atm_close() would not
> fix the race.
>
> The RCU approach with synchronize_rcu() ensures ALL callers
> including mld_ifc_work have finished before priv->lecd is
> cleared.
>
After another look the uaf [1] is due to the race
vcc_release() mld_sendpack()
--- ---
ip6_output()
__dev_queue_xmit()
rcu_read_lock_bh(); // rcu section
__dev_xmit_skb()
netdev_start_xmit()
lec_start_xmit()
if (!priv->lecd) { // check lecd
kfree_skb(skb);
return NETDEV_TX_OK;
}
vcc_destroy_socket()
vcc->dev->ops->close(vcc);
lec_atm_close()
priv->lecd = NULL; // clear lecd
netif_stop_queue(dev);
sock_put() // free sock
lec_arp_resolve()
send_to_lecd()
sock_def_readable() // uaf
and syncing rcu after clearing lecd is the correct fix because lecd is checked
with rcu lock held.
[1] Subject: [syzbot] [net?] KASAN: slab-use-after-free Read in sock_def_readable (2)
https://lore.kernel.org/all/69ad7ccb.a00a0220.b130.0003.GAE@google.com/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] atm: lec: fix use-after-free in sock_def_readable()
2026-03-09 15:59 [PATCH v2] atm: lec: fix use-after-free in sock_def_readable() Deepanshu Kartikey
2026-03-10 3:18 ` Hillf Danton
@ 2026-03-14 12:00 ` Deepanshu Kartikey
2026-03-14 12:10 ` Eric Dumazet
2026-03-14 15:40 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 7+ messages in thread
From: Deepanshu Kartikey @ 2026-03-14 12:00 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, horms
Cc: mingo, tglx, jiayuan.chen, kees, netdev, dan.carpenter,
linux-kernel, syzbot+f50072212ab792c86925
On Mon, Mar 9, 2026 at 9:29 PM Deepanshu Kartikey <kartikey406@gmail.com> wrote:
>
> A race condition exists between lec_atm_close() setting priv->lecd
> to 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
> any synchronization, while callers dereference priv->lecd without
> any protection against concurrent teardown.
>
> Fix this by converting priv->lecd to an RCU-protected pointer:
> - Mark priv->lecd as __rcu in lec.h
> - Use rcu_assign_pointer() in lec_atm_close() and lecd_attach()
> for safe pointer assignment
> - Use rcu_access_pointer() for NULL checks that do not dereference
> the pointer in lec_start_xmit(), lec_push(), send_to_lecd() and
> lecd_attach()
> - Use rcu_read_lock/rcu_dereference/rcu_read_unlock in send_to_lecd(),
> lec_handle_bridge() and lec_atm_send() to safely access lecd
> - Use rcu_assign_pointer() followed by synchronize_rcu() in
> lec_atm_close() to ensure all readers have completed before
> proceeding. This is safe since lec_atm_close() is called from
> vcc_release() which holds lock_sock(), a sleeping lock.
> - Remove the manual sk_receive_queue drain from lec_atm_close()
> since vcc_destroy_socket() already drains it after lec_atm_close()
> returns.
>
> v2: Switch from spinlock + sock_hold/put approach to RCU to properly
> fix the race. The v1 spinlock approach had two issues pointed out
> by Eric Dumazet:
> 1. priv->lecd was still accessed directly after releasing the
> lock instead of using a local copy.
> 2. The spinlock did not prevent packets being queued after
> lec_atm_close() drains sk_receive_queue since timer and
> workqueue paths bypass netif_stop_queue().
>
> Note: Syzbot patch testing was attempted but the test VM terminated
> unexpectedly with "Connection to localhost closed by remote host",
> likely due to a QEMU AHCI emulation issue unrelated to this fix.
> 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
> Link: https://lore.kernel.org/all/20260309093614.502094-1-kartikey406@gmail.com/T/ [v1]
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> net/atm/lec.c | 70 +++++++++++++++++++++++++++++++++------------------
> net/atm/lec.h | 2 +-
> 2 files changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index fb93c6e1c329..32adf28663a0 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -154,10 +154,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);
> - sk = sk_atm(priv->lecd);
> - skb_queue_tail(&sk->sk_receive_queue, skb2);
> - sk->sk_data_ready(sk);
> + struct atm_vcc *vcc;
> +
> + rcu_read_lock();
> + vcc = rcu_dereference(priv->lecd);
> + if (vcc) {
> + atm_force_charge(vcc, skb2->truesize);
> + sk = sk_atm(vcc);
> + skb_queue_tail(&sk->sk_receive_queue, skb2);
> + sk->sk_data_ready(sk);
> + } else {
> + dev_kfree_skb(skb2);
> + }
> + rcu_read_unlock();
> }
> }
> #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -216,7 +225,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
> int is_rdesc;
>
> pr_debug("called\n");
> - if (!priv->lecd) {
> + if (!rcu_access_pointer(priv->lecd)) {
> pr_info("%s:No lecd attached\n", dev->name);
> dev->stats.tx_errors++;
> netif_stop_queue(dev);
> @@ -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);
> - sk = sk_atm(priv->lecd);
> - skb_queue_tail(&sk->sk_receive_queue, skb2);
> - sk->sk_data_ready(sk);
> + struct atm_vcc *vcc;
> +
> + rcu_read_lock();
> + vcc = rcu_dereference(priv->lecd);
> + if (vcc) {
> + atm_force_charge(vcc, skb2->truesize);
> + sk = sk_atm(vcc);
> + skb_queue_tail(&sk->sk_receive_queue, skb2);
> + sk->sk_data_ready(sk);
> + } else {
> + dev_kfree_skb(skb2);
> + }
> + rcu_read_unlock();
> }
> }
> #endif /* IS_ENABLED(CONFIG_BRIDGE) */
> @@ -468,23 +486,16 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
>
> 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);
>
> - priv->lecd = NULL;
> + rcu_assign_pointer(priv->lecd, NULL);
> + synchronize_rcu();
> /* Do something needful? */
>
> netif_stop_queue(dev);
> lec_arp_destroy(priv);
>
> - if (skb_peek(&sk_atm(vcc)->sk_receive_queue))
> - pr_info("%s closing with messages pending\n", dev->name);
> - while ((skb = skb_dequeue(&sk_atm(vcc)->sk_receive_queue))) {
> - atm_return(vcc, skb->truesize);
> - dev_kfree_skb(skb);
> - }
> -
> pr_info("%s: Shut down!\n", dev->name);
> module_put(THIS_MODULE);
> }
> @@ -510,12 +521,14 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
> const unsigned char *mac_addr, const unsigned char *atm_addr,
> struct sk_buff *data)
> {
> + struct atm_vcc *vcc;
> struct sock *sk;
> struct sk_buff *skb;
> struct atmlec_msg *mesg;
>
> - if (!priv || !priv->lecd)
> + if (!priv || !rcu_access_pointer(priv->lecd))
> return -1;
> +
> skb = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
> if (!skb)
> return -1;
> @@ -532,18 +545,27 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
> if (atm_addr)
> memcpy(&mesg->content.normal.atm_addr, atm_addr, ATM_ESA_LEN);
>
> - atm_force_charge(priv->lecd, skb->truesize);
> - sk = sk_atm(priv->lecd);
> + rcu_read_lock();
> + vcc = rcu_dereference(priv->lecd);
> + if (!vcc) {
> + rcu_read_unlock();
> + kfree_skb(skb);
> + return -1;
> + }
> +
> + atm_force_charge(vcc, skb->truesize);
> + sk = sk_atm(vcc);
> skb_queue_tail(&sk->sk_receive_queue, skb);
> sk->sk_data_ready(sk);
>
> if (data != NULL) {
> pr_debug("about to send %d bytes of data\n", data->len);
> - atm_force_charge(priv->lecd, data->truesize);
> + atm_force_charge(vcc, data->truesize);
> skb_queue_tail(&sk->sk_receive_queue, data);
> sk->sk_data_ready(sk);
> }
>
> + rcu_read_unlock();
> return 0;
> }
>
> @@ -618,7 +640,7 @@ static void lec_push(struct atm_vcc *vcc, struct sk_buff *skb)
>
> atm_return(vcc, skb->truesize);
> if (*(__be16 *) skb->data == htons(priv->lecid) ||
> - !priv->lecd || !(dev->flags & IFF_UP)) {
> + !rcu_access_pointer(priv->lecd) || !(dev->flags & IFF_UP)) {
> /*
> * Probably looping back, or if lecd is missing,
> * lecd has gone down
> @@ -753,12 +775,12 @@ static int lecd_attach(struct atm_vcc *vcc, int arg)
> priv = netdev_priv(dev_lec[i]);
> } else {
> priv = netdev_priv(dev_lec[i]);
> - if (priv->lecd)
> + if (rcu_access_pointer(priv->lecd))
> return -EADDRINUSE;
> }
> lec_arp_init(priv);
> priv->itfnum = i; /* LANE2 addition */
> - priv->lecd = vcc;
> + rcu_assign_pointer(priv->lecd, vcc);
> vcc->dev = &lecatm_dev;
> vcc_insert_socket(sk_atm(vcc));
>
> diff --git a/net/atm/lec.h b/net/atm/lec.h
> index be0e2667bd8c..ec85709bf818 100644
> --- a/net/atm/lec.h
> +++ b/net/atm/lec.h
> @@ -91,7 +91,7 @@ struct lec_priv {
> */
> spinlock_t lec_arp_lock;
> struct atm_vcc *mcast_vcc; /* Default Multicast Send VCC */
> - struct atm_vcc *lecd;
> + struct atm_vcc __rcu *lecd;
> struct delayed_work lec_arp_work; /* C10 */
> unsigned int maximum_unknown_frame_count;
> /*
> --
> 2.43.0
>
Hi Eric,
Gentle ping on the v2 patch below.
Please let me know if you have any concerns.
Thanks,
Deepanshu Kartikey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] atm: lec: fix use-after-free in sock_def_readable()
2026-03-14 12:00 ` Deepanshu Kartikey
@ 2026-03-14 12:10 ` Eric Dumazet
0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2026-03-14 12:10 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: davem, kuba, pabeni, horms, mingo, tglx, jiayuan.chen, kees,
netdev, dan.carpenter, linux-kernel, syzbot+f50072212ab792c86925
On Sat, Mar 14, 2026 at 1:00 PM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> Hi Eric,
>
> Gentle ping on the v2 patch below.
> Please let me know if you have any concerns.
>
> Thanks,
> Deepanshu Kartikey
Patch LGTM, thanks.
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] atm: lec: fix use-after-free in sock_def_readable()
2026-03-09 15:59 [PATCH v2] atm: lec: fix use-after-free in sock_def_readable() Deepanshu Kartikey
2026-03-10 3:18 ` Hillf Danton
2026-03-14 12:00 ` Deepanshu Kartikey
@ 2026-03-14 15:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-03-14 15:40 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: davem, edumazet, kuba, pabeni, horms, mingo, tglx, jiayuan.chen,
kees, netdev, dan.carpenter, linux-kernel,
syzbot+f50072212ab792c86925
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 9 Mar 2026 21:29:08 +0530 you wrote:
> A race condition exists between lec_atm_close() setting priv->lecd
> to 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
> any synchronization, while callers dereference priv->lecd without
> any protection against concurrent teardown.
>
> [...]
Here is the summary with links:
- [v2] atm: lec: fix use-after-free in sock_def_readable()
https://git.kernel.org/netdev/net/c/922814879542
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-14 15:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-09 15:59 [PATCH v2] atm: lec: fix use-after-free in sock_def_readable() Deepanshu Kartikey
2026-03-10 3:18 ` Hillf Danton
2026-03-12 1:03 ` Deepanshu Kartikey
2026-03-14 2:53 ` Hillf Danton
2026-03-14 12:00 ` Deepanshu Kartikey
2026-03-14 12:10 ` Eric Dumazet
2026-03-14 15:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox