[PATCH] net: mctp: hold route device before source address lookup

[PATCH] net: mctp: hold route device before source address lookup

[Yiqi Sun]

mctp_route_lookup() walks the route table under RCU and may find a direct
route whose route-table reference to rt->dev is being removed concurrently
by NETDEV_UNREGISTER.

Since commit 22cb45afd221 ("net: mctp: perform source address lookups
when we populate our dst"), the direct-route lookup path reads the source
address with mctp_dev_saddr(rt->dev) before mctp_dst_from_route() takes the
dst's own mctp_dev reference. If device teardown wins that race,
mctp_route_remove_dev() can delete the route and mctp_route_release() can
drop the route's mctp_dev reference to zero. mctp_dev_put() frees
mdev->addrs synchronously, while only the mctp_dev body is deferred with
kfree_rcu().

That leaves the lookup side able to read freed mdev->addrs in
mctp_dev_saddr(), or later increment a zero refcount in mctp_dev_hold().

Add a try-hold helper for mctp_dev and use it before reading the source
address from a direct route. mctp_dst_from_route() now consumes that held
reference when it populates dst. If no dst is requested, drop the temporary
reference in mctp_route_lookup(). Do the same when the route cannot be used
for a gateway path without a source address.

Fixes: 22cb45afd221 ("net: mctp: perform source address lookups when we populate our dst")
Signed-off-by: Yiqi Sun <sunyiqixm@gmail.com>
---
 include/net/mctpdevice.h |  1 +
 net/mctp/device.c        |  5 +++++
 net/mctp/route.c         | 28 ++++++++++++++++++++--------
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/include/net/mctpdevice.h b/include/net/mctpdevice.h
index 957d9ef924c5..1d052f11127c 100644
--- a/include/net/mctpdevice.h
+++ b/include/net/mctpdevice.h
@@ -50,6 +50,7 @@ int mctp_register_netdev(struct net_device *dev,
 void mctp_unregister_netdev(struct net_device *dev);
 
 void mctp_dev_hold(struct mctp_dev *mdev);
+bool mctp_dev_try_hold(struct mctp_dev *mdev);
 void mctp_dev_put(struct mctp_dev *mdev);
 
 void mctp_dev_set_key(struct mctp_dev *dev, struct mctp_sk_key *key);
diff --git a/net/mctp/device.c b/net/mctp/device.c
index 2c84df674669..18b192bbfaa7 100644
--- a/net/mctp/device.c
+++ b/net/mctp/device.c
@@ -303,6 +303,11 @@ void mctp_dev_hold(struct mctp_dev *mdev)
 	refcount_inc(&mdev->refs);
 }
 
+bool mctp_dev_try_hold(struct mctp_dev *mdev)
+{
+	return mdev && refcount_inc_not_zero(&mdev->refs);
+}
+
 void mctp_dev_put(struct mctp_dev *mdev)
 {
 	if (mdev && refcount_dec_and_test(&mdev->refs)) {
diff --git a/net/mctp/route.c b/net/mctp/route.c
index 1f3dccbb7aed..3d0737030917 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -897,15 +897,16 @@ static mctp_eid_t mctp_dev_saddr(struct mctp_dev *dev)
 	return addr;
 }
 
-/* must only be called on a direct route, as the final output hop */
+/* must only be called on a direct route, as the final output hop, with a
+ * reference already held on route->dev.
+ */
 static void mctp_dst_from_route(struct mctp_dst *dst, mctp_eid_t eid,
 				mctp_eid_t saddr, unsigned int mtu,
-				struct mctp_route *route)
+				struct mctp_route *route, struct mctp_dev *dev)
 {
-	mctp_dev_hold(route->dev);
 	dst->nexthop = eid;
-	dst->dev = route->dev;
-	dst->mtu = READ_ONCE(dst->dev->dev->mtu);
+	dst->dev = dev;
+	dst->mtu = READ_ONCE(dev->dev->mtu);
 	if (mtu)
 		dst->mtu = min(dst->mtu, mtu);
 	dst->halen = 0;
@@ -998,14 +999,25 @@ 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);
+			struct mctp_dev *dev = rt->dev;
+			mctp_eid_t saddr;
+
+			if (!mctp_dev_try_hold(dev))
+				break;
+
+			saddr = mctp_dev_saddr(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(dev);
 				break;
+			}
 
 			if (dst)
-				mctp_dst_from_route(dst, daddr, saddr, mtu, rt);
+				mctp_dst_from_route(dst, daddr, saddr, mtu, rt,
+						    dev);
+			else
+				mctp_dev_put(dev);
 			rc = 0;
 			break;
 
-- 
2.34.1