Forwarded: [PATCH] atm: lec: fix use-after-free in sock_def_readable()

[syzbot <syzbot+f50072212ab792c86925@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] atm: lec: fix use-after-free in sock_def_readable()
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master


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.

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.0