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

diff --git a/net/atm/lec.c b/net/atm/lec.c
index fb93c6e1c32942c691cf80d06a93213fa92e6757..10e260acf6027c95f17219eeb208c8d088608a57 100644 (file)
--- 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 +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));
 

diff --git a/net/atm/lec.h b/net/atm/lec.h
index be0e2667bd8c3f5474771a20de2b138d90523d62..ec85709bf81859e00f93f89f24560501662fabe3 100644 (file)
--- 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;
                                                /*