mirror of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-03-21 23:16:50 +08:00
atm: lec: fix use-after-free in sock_def_readable()
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>
This commit is contained in:
committed by
Jakub Kicinski
parent
99600f79b2
commit
9228148795
@@ -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 +458,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));
|
||||
|
||||
|
||||
@@ -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;
|
||||
/*
|
||||
|
||||
Reference in New Issue
Block a user