Skip to content

Commit abc10f8

Browse files
deepanshu406gregkh
authored andcommitted
atm: lec: fix use-after-free in sock_def_readable()
[ Upstream commit 9228148 ] 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> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20260309155908.508768-1-kartikey406@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent c8dc23c commit abc10f8

2 files changed

Lines changed: 48 additions & 26 deletions

File tree

net/atm/lec.c

Lines changed: 47 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,19 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
154154
/* 0x01 is topology change */
155155

156156
priv = netdev_priv(dev);
157-
atm_force_charge(priv->lecd, skb2->truesize);
158-
sk = sk_atm(priv->lecd);
159-
skb_queue_tail(&sk->sk_receive_queue, skb2);
160-
sk->sk_data_ready(sk);
157+
struct atm_vcc *vcc;
158+
159+
rcu_read_lock();
160+
vcc = rcu_dereference(priv->lecd);
161+
if (vcc) {
162+
atm_force_charge(vcc, skb2->truesize);
163+
sk = sk_atm(vcc);
164+
skb_queue_tail(&sk->sk_receive_queue, skb2);
165+
sk->sk_data_ready(sk);
166+
} else {
167+
dev_kfree_skb(skb2);
168+
}
169+
rcu_read_unlock();
161170
}
162171
}
163172
#endif /* IS_ENABLED(CONFIG_BRIDGE) */
@@ -216,7 +225,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
216225
int is_rdesc;
217226

218227
pr_debug("called\n");
219-
if (!priv->lecd) {
228+
if (!rcu_access_pointer(priv->lecd)) {
220229
pr_info("%s:No lecd attached\n", dev->name);
221230
dev->stats.tx_errors++;
222231
netif_stop_queue(dev);
@@ -449,10 +458,19 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
449458
break;
450459
skb2->len = sizeof(struct atmlec_msg);
451460
skb_copy_to_linear_data(skb2, mesg, sizeof(*mesg));
452-
atm_force_charge(priv->lecd, skb2->truesize);
453-
sk = sk_atm(priv->lecd);
454-
skb_queue_tail(&sk->sk_receive_queue, skb2);
455-
sk->sk_data_ready(sk);
461+
struct atm_vcc *vcc;
462+
463+
rcu_read_lock();
464+
vcc = rcu_dereference(priv->lecd);
465+
if (vcc) {
466+
atm_force_charge(vcc, skb2->truesize);
467+
sk = sk_atm(vcc);
468+
skb_queue_tail(&sk->sk_receive_queue, skb2);
469+
sk->sk_data_ready(sk);
470+
} else {
471+
dev_kfree_skb(skb2);
472+
}
473+
rcu_read_unlock();
456474
}
457475
}
458476
#endif /* IS_ENABLED(CONFIG_BRIDGE) */
@@ -468,23 +486,16 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
468486

469487
static void lec_atm_close(struct atm_vcc *vcc)
470488
{
471-
struct sk_buff *skb;
472489
struct net_device *dev = (struct net_device *)vcc->proto_data;
473490
struct lec_priv *priv = netdev_priv(dev);
474491

475-
priv->lecd = NULL;
492+
rcu_assign_pointer(priv->lecd, NULL);
493+
synchronize_rcu();
476494
/* Do something needful? */
477495

478496
netif_stop_queue(dev);
479497
lec_arp_destroy(priv);
480498

481-
if (skb_peek(&sk_atm(vcc)->sk_receive_queue))
482-
pr_info("%s closing with messages pending\n", dev->name);
483-
while ((skb = skb_dequeue(&sk_atm(vcc)->sk_receive_queue))) {
484-
atm_return(vcc, skb->truesize);
485-
dev_kfree_skb(skb);
486-
}
487-
488499
pr_info("%s: Shut down!\n", dev->name);
489500
module_put(THIS_MODULE);
490501
}
@@ -510,12 +521,14 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
510521
const unsigned char *mac_addr, const unsigned char *atm_addr,
511522
struct sk_buff *data)
512523
{
524+
struct atm_vcc *vcc;
513525
struct sock *sk;
514526
struct sk_buff *skb;
515527
struct atmlec_msg *mesg;
516528

517-
if (!priv || !priv->lecd)
529+
if (!priv || !rcu_access_pointer(priv->lecd))
518530
return -1;
531+
519532
skb = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
520533
if (!skb)
521534
return -1;
@@ -532,18 +545,27 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
532545
if (atm_addr)
533546
memcpy(&mesg->content.normal.atm_addr, atm_addr, ATM_ESA_LEN);
534547

535-
atm_force_charge(priv->lecd, skb->truesize);
536-
sk = sk_atm(priv->lecd);
548+
rcu_read_lock();
549+
vcc = rcu_dereference(priv->lecd);
550+
if (!vcc) {
551+
rcu_read_unlock();
552+
kfree_skb(skb);
553+
return -1;
554+
}
555+
556+
atm_force_charge(vcc, skb->truesize);
557+
sk = sk_atm(vcc);
537558
skb_queue_tail(&sk->sk_receive_queue, skb);
538559
sk->sk_data_ready(sk);
539560

540561
if (data != NULL) {
541562
pr_debug("about to send %d bytes of data\n", data->len);
542-
atm_force_charge(priv->lecd, data->truesize);
563+
atm_force_charge(vcc, data->truesize);
543564
skb_queue_tail(&sk->sk_receive_queue, data);
544565
sk->sk_data_ready(sk);
545566
}
546567

568+
rcu_read_unlock();
547569
return 0;
548570
}
549571

@@ -618,7 +640,7 @@ static void lec_push(struct atm_vcc *vcc, struct sk_buff *skb)
618640

619641
atm_return(vcc, skb->truesize);
620642
if (*(__be16 *) skb->data == htons(priv->lecid) ||
621-
!priv->lecd || !(dev->flags & IFF_UP)) {
643+
!rcu_access_pointer(priv->lecd) || !(dev->flags & IFF_UP)) {
622644
/*
623645
* Probably looping back, or if lecd is missing,
624646
* lecd has gone down
@@ -753,12 +775,12 @@ static int lecd_attach(struct atm_vcc *vcc, int arg)
753775
priv = netdev_priv(dev_lec[i]);
754776
} else {
755777
priv = netdev_priv(dev_lec[i]);
756-
if (priv->lecd)
778+
if (rcu_access_pointer(priv->lecd))
757779
return -EADDRINUSE;
758780
}
759781
lec_arp_init(priv);
760782
priv->itfnum = i; /* LANE2 addition */
761-
priv->lecd = vcc;
783+
rcu_assign_pointer(priv->lecd, vcc);
762784
vcc->dev = &lecatm_dev;
763785
vcc_insert_socket(sk_atm(vcc));
764786

net/atm/lec.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ struct lec_priv {
9191
*/
9292
spinlock_t lec_arp_lock;
9393
struct atm_vcc *mcast_vcc; /* Default Multicast Send VCC */
94-
struct atm_vcc *lecd;
94+
struct atm_vcc __rcu *lecd;
9595
struct delayed_work lec_arp_work; /* C10 */
9696
unsigned int maximum_unknown_frame_count;
9797
/*

0 commit comments

Comments
 (0)