[PATCH net] appletalk: fix use-after-free in atalk_find_primary()

[Yizhou Zhao]

atalk_find_primary() walks the global AppleTalk interface list under
atalk_interfaces_lock, but returns a pointer to iface->address after
dropping that lock.  Both atalk_autobind() and atalk_bind() then
dereference the returned pointer without any lifetime protection.

The interface can be removed concurrently through the normal AppleTalk
interface ioctl path.  SIOCATALKDIFADDR calls atalk_dev_down(), which
eventually reaches atif_drop_device() and frees the same struct
atalk_iface that owns the returned address field.  A racing bind can
therefore read from freed memory.

This is reachable with a configured AppleTalk interface; reproducing the
race does not require a malicious device or driver.  The configuration
ioctls require CAP_NET_ADMIN in the initial user namespace, and
AF_APPLETALK sockets are limited to init_net.

Fix the lifetime issue without changing the returned address pointer
type.  Rename the helper to atalk_find_primary_locked() and keep
atalk_interfaces_lock held across the return.  The callers now copy
s_net and s_node while the lock is still held, then immediately release
the lock before doing any further work.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@xxxxxxxxxxxxxxx
Reported-by: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>
Reported-by: Yuxiang Yang <yangyx22@xxxxxxxxxxxxxxxxxxxxx>
Reported-by: Ao Wang <wangao@xxxxxxxxxx>
Reported-by: Xuewei Feng <fengxw06@xxxxxxx>
Reported-by: Qi Li <qli01@xxxxxxxxxxxxxxx>
Reported-by: Ke Xu <xuke@xxxxxxxxxxxxxxx>
Assisted-by: GLM:GLM-5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>
---
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 30a6dc06291c..4d6576cd0ae8 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -351,7 +351,7 @@ struct atalk_addr *atalk_find_dev_addr(struct net_device *dev)
 	return iface ? &iface->address : NULL;
 }
 
-static struct atalk_addr *atalk_find_primary(void)
+static struct atalk_addr *atalk_find_primary_locked(void)
 {
 	struct atalk_iface *fiface = NULL;
 	struct atalk_addr *retval;
@@ -378,7 +378,6 @@ static struct atalk_addr *atalk_find_primary(void)
 	else
 		retval = NULL;
 out:
-	read_unlock_bh(&atalk_interfaces_lock);
 	return retval;
 }
 
@@ -1132,20 +1131,24 @@ static int atalk_autobind(struct sock *sk)
 {
 	struct atalk_sock *at = at_sk(sk);
 	struct sockaddr_at sat;
-	struct atalk_addr *ap = atalk_find_primary();
+	struct atalk_addr *ap = atalk_find_primary_locked();
 	int n = -EADDRNOTAVAIL;
 
 	if (!ap || ap->s_net == htons(ATADDR_ANYNET))
-		goto out;
+		goto unlock_and_out;
 
 	at->src_net  = sat.sat_addr.s_net  = ap->s_net;
 	at->src_node = sat.sat_addr.s_node = ap->s_node;
+	read_unlock_bh(&atalk_interfaces_lock);
 
 	n = atalk_pick_and_bind_port(sk, &sat);
 	if (!n)
 		sock_reset_flag(sk, SOCK_ZAPPED);
 out:
 	return n;
+unlock_and_out:
+	read_unlock_bh(&atalk_interfaces_lock);
+	goto out;
 }
 
 /* Set the address 'our end' of the connection */
@@ -1165,14 +1168,15 @@ static int atalk_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int a
 
 	lock_sock(sk);
 	if (addr->sat_addr.s_net == htons(ATADDR_ANYNET)) {
-		struct atalk_addr *ap = atalk_find_primary();
+		struct atalk_addr *ap = atalk_find_primary_locked();
 
 		err = -EADDRNOTAVAIL;
 		if (!ap)
-			goto out;
+			goto unlock_and_out;
 
 		at->src_net  = addr->sat_addr.s_net = ap->s_net;
 		at->src_node = addr->sat_addr.s_node = ap->s_node;
+		read_unlock_bh(&atalk_interfaces_lock);
 	} else {
 		err = -EADDRNOTAVAIL;
 		if (!atalk_find_interface(addr->sat_addr.s_net,
@@ -1201,6 +1205,9 @@ static int atalk_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int a
 out:
 	release_sock(sk);
 	return err;
+unlock_and_out:
+	read_unlock_bh(&atalk_interfaces_lock);
+	goto out;
 }
 
 /* Set the address we talk to */

--
2.43.0