[PATCH net-next] net: mctp: defer creation of dst after source-address check

[PATCH net-next] net: mctp: defer creation of dst after source-address check

[Jeremy Kerr]

Sashiko reports:

> mctp_dst_from_route() increments the device reference count by calling
> mctp_dev_hold(). When a valid route is found and dst is NULL, the
> structure copy is bypassed and rc is set to 0.

Instead of optimistically creating a dst from the final route (then
releasing it if the saddr is invalid), perform the saddr check first.

This means we don't have an unuecessary hold/release on the dev, which
could leak if the dst pointer is NULL. No caller passes a NULL dst at
present though (so the leak is not possible), but this is an intended
use of mctp_dst_from_route().

Fixes: 22cb45afd221 ("net: mctp: perform source address lookups when we populate our dst")
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 net/mctp/route.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/mctp/route.c b/net/mctp/route.c
index 78263e7ae423..26fb8c6bbad2 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -897,7 +897,8 @@ static mctp_eid_t mctp_dev_saddr(struct mctp_dev *dev)
 
 /* must only be called on a direct route, as the final output hop */
 static void mctp_dst_from_route(struct mctp_dst *dst, mctp_eid_t eid,
-				unsigned int mtu, struct mctp_route *route)
+				mctp_eid_t saddr, unsigned int mtu,
+				struct mctp_route *route)
 {
 	mctp_dev_hold(route->dev);
 	dst->nexthop = eid;
@@ -907,7 +908,7 @@ static void mctp_dst_from_route(struct mctp_dst *dst, mctp_eid_t eid,
 		dst->mtu = min(dst->mtu, mtu);
 	dst->halen = 0;
 	dst->output = route->output;
-	dst->saddr = mctp_dev_saddr(route->dev);
+	dst->saddr = saddr;
 }
 
 int mctp_dst_from_extaddr(struct mctp_dst *dst, struct net *net, int ifindex,
@@ -975,7 +976,6 @@ int mctp_route_lookup(struct net *net, unsigned int dnet,
 {
 	const unsigned int max_depth = 32;
 	unsigned int depth, mtu = 0;
-	struct mctp_dst dst_tmp;
 	int rc = -EHOSTUNREACH;
 
 	rcu_read_lock();
@@ -996,15 +996,15 @@ int mctp_route_lookup(struct net *net, unsigned int dnet,
 			mtu = mtu ?: rt->mtu;
 
 		if (rt->dst_type == MCTP_ROUTE_DIRECT) {
-			mctp_dst_from_route(&dst_tmp, daddr, mtu, rt);
+			mctp_eid_t saddr = mctp_dev_saddr(rt->dev);
+
 			/* cannot do gateway-ed routes without a src  */
-			if (dst_tmp.saddr == MCTP_ADDR_NULL && depth != 0) {
-				mctp_dst_release(&dst_tmp);
-			} else {
-				if (dst)
-					*dst = dst_tmp;
-				rc = 0;
-			}
+			if (saddr == MCTP_ADDR_NULL && depth != 0)
+				break;
+
+			if (dst)
+				mctp_dst_from_route(dst, daddr, saddr, mtu, rt);
+			rc = 0;
 			break;
 
 		} else if (rt->dst_type == MCTP_ROUTE_GATEWAY) {

---
base-commit: 8b0e64d6c9e7feec5ba5643b4fa8b7fd54464778
change-id: 20260403-dev-mctp-dst-defer-93e88da84ed9

Best regards,
--  
Jeremy Kerr <jk@codeconstruct.com.au>

Re: [PATCH net-next] net: mctp: defer creation of dst after source-address check

[Simon Horman]

On Fri, Apr 03, 2026 at 10:24:51AM +0800, Jeremy Kerr wrote:
> Sashiko reports:
> 
> > mctp_dst_from_route() increments the device reference count by calling
> > mctp_dev_hold(). When a valid route is found and dst is NULL, the
> > structure copy is bypassed and rc is set to 0.
> 
> Instead of optimistically creating a dst from the final route (then
> releasing it if the saddr is invalid), perform the saddr check first.
> 
> This means we don't have an unuecessary hold/release on the dev, which
> could leak if the dst pointer is NULL. No caller passes a NULL dst at
> present though (so the leak is not possible), but this is an intended
> use of mctp_dst_from_route().
> 
> Fixes: 22cb45afd221 ("net: mctp: perform source address lookups when we populate our dst")
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next] net: mctp: defer creation of dst after source-address check

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 03 Apr 2026 10:24:51 +0800 you wrote:
> Sashiko reports:
> 
> > mctp_dst_from_route() increments the device reference count by calling
> > mctp_dev_hold(). When a valid route is found and dst is NULL, the
> > structure copy is bypassed and rc is set to 0.
> 
> Instead of optimistically creating a dst from the final route (then
> releasing it if the saddr is invalid), perform the saddr check first.
> 
> [...]

Here is the summary with links:
  - [net-next] net: mctp: defer creation of dst after source-address check
    https://git.kernel.org/netdev/net-next/c/f32ba0963119

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html