[PATCH] net: mctp: hold a reference to the route device in mctp_route_lookup()

Aldo Ariel Panzardo <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
mctp_route_lookup() uses rt->dev without holding a reference on it.
mctp_route_lookup_single() returns the route under RCU only, so the
route's device can be torn down concurrently: mctp_dev_put() drops the
last reference and synchronously kfree()s mdev->addrs.  mctp_dev_saddr()
then reads rt->dev->addrs[0], giving a use-after-free reachable by an
unprivileged local AF_MCTP user on the receive/forwarding path (no
CAP_NET_RAW required):

  BUG: KASAN: slab-use-after-free in mctp_route_lookup
  Read of size 1 at addr ... by task mctp_uaf/...
   mctp_route_lookup
   mctp_pkttype_receive
  Freed by task ...:
   kfree
   mctp_dev_put
   mctp_dev_notify

In the same window mctp_dst_from_route() -> mctp_dev_hold() also
increments a refcount that has already reached zero
("refcount_t: addition on 0 ... mctp_dev_hold").

This reintroduces the use-after-free class of CVE-2023-3439: the source
address lookup was moved ahead of the point where the destination takes
its device reference.

Take a reference with refcount_inc_not_zero() before touching rt->dev,
skip a device that is already dead, and drop the reference once the
destination has taken its own.

Fixes: 22cb45afd221 ("net: mctp: perform source address lookups when we populate our dst")
Cc: [email protected]
Signed-off-by: Aldo Ariel Panzardo <[email protected]>
---
A reproducer is available on request.

 net/mctp/route.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/net/mctp/route.c b/net/mctp/route.c
index 1f3dccbb7a..b19c63a569 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -998,14 +998,29 @@ int mctp_route_lookup(struct net *net, unsigned int dnet,
 			mtu = mtu ?: rt->mtu;
 
 		if (rt->dst_type == MCTP_ROUTE_DIRECT) {
-			mctp_eid_t saddr = mctp_dev_saddr(rt->dev);
+			mctp_eid_t saddr;
+
+			/* rt->dev may be going away concurrently: its last
+			 * reference is dropped in mctp_dev_put(), which frees
+			 * mdev->addrs that mctp_dev_saddr() reads, and
+			 * mctp_dst_from_route() takes a reference on it.  Pin
+			 * it before use, and skip a device that is already
+			 * dead rather than resurrecting it.
+			 */
+			if (!refcount_inc_not_zero(&rt->dev->refs))
+				break;
+
+			saddr = mctp_dev_saddr(rt->dev);
 
 			/* cannot do gateway-ed routes without a src  */
-			if (saddr == MCTP_ADDR_NULL && depth != 0)
+			if (saddr == MCTP_ADDR_NULL && depth != 0) {
+				mctp_dev_put(rt->dev);
 				break;
+			}
 
 			if (dst)
 				mctp_dst_from_route(dst, daddr, saddr, mtu, rt);
+			mctp_dev_put(rt->dev);
 			rc = 0;
 			break;
 
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.