netdev - Re: [PATCH net-next 12/13] net: mctp: add gateway routing support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250613171137.GM414686@horms.kernel.org>
Date: Fri, 13 Jun 2025 18:11:37 +0100
From: Simon Horman <horms@...nel.org>
To: Jeremy Kerr <jk@...econstruct.com.au>
Cc: Matt Johnston <matt@...econstruct.com.au>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 12/13] net: mctp: add gateway routing support

On Wed, Jun 11, 2025 at 02:30:39PM +0800, Jeremy Kerr wrote:
> This change allows for gateway routing, where a route table entry
> may reference a routable endpoint (by network and EID), instead of
> routing directly to a netdevice.
> 
> We add support for a RTM_GATEWAY attribute for netlink route updates,
> with an attribute format of:
> 
>     struct mctp_fq_addr {
>         unsigned int net;
>         mctp_eid_t eid;
>     }
> 
> - we need the net here to uniquely identify the target EID, as we no
> longer have the device reference directly (which would provide the net
> id in the case of direct routes).
> 
> This makes route lookups recursive, as a route lookup that returns a
> gateway route must be resolved into a direct route (ie, to a device)
> eventually. We provide a limit to the route lookups, to prevent infinite
> loop routing.
> 
> The route lookup populates a new 'nexthop' field in the dst structure,
> which now specifies the key for the neighbour table lookup on device
> output, rather than using the packet destination address directly.
> 
> Signed-off-by: Jeremy Kerr <jk@...econstruct.com.au>

...

> diff --git a/net/mctp/route.c b/net/mctp/route.c

...

> -/* base parsing; common to both _lookup and _populate variants */
> +/* base parsing; common to both _lookup and _populate variants.
> + *
> + * For gateway routes (which have a RTA_GATEWAY, and no RTA_OIF), we populate
> + * *gatweayp. for direct routes (RTA_OIF, no RTA_GATEWAY), we populate *mdev.
> + */
>  static int mctp_route_nlparse_common(struct net *net, struct nlmsghdr *nlh,
>  				     struct netlink_ext_ack *extack,
>  				     struct nlattr **tb, struct rtmsg **rtm,
>  				     struct mctp_dev **mdev,
> +				     struct mctp_fq_addr *gatewayp,
>  				     mctp_eid_t *daddr_start)
>  {
> +	struct mctp_fq_addr *gateway;
> +	unsigned int ifindex = 0;
>  	struct net_device *dev;
> -	unsigned int ifindex;
>  	int rc;
>  
>  	rc = nlmsg_parse(nlh, sizeof(struct rtmsg), tb, RTA_MAX,
> @@ -1321,11 +1372,44 @@ static int mctp_route_nlparse_common(struct net *net, struct nlmsghdr *nlh,
>  	}
>  	*daddr_start = nla_get_u8(tb[RTA_DST]);
>  
> -	if (!tb[RTA_OIF]) {
> -		NL_SET_ERR_MSG(extack, "ifindex missing");
> +	if (tb[RTA_OIF])
> +		ifindex = nla_get_u32(tb[RTA_OIF]);
> +
> +	if (tb[RTA_GATEWAY])
> +		gateway = nla_data(tb[RTA_GATEWAY]);
> +
> +	if (ifindex && gateway) {

Hi Jeremy,

gateway may be uninitialised here...

> +		NL_SET_ERR_MSG(extack,
> +			       "cannot specify both ifindex and gateway");
> +		return -EINVAL;
> +
> +	} else if (ifindex) {
> +		dev = __dev_get_by_index(net, ifindex);
> +		if (!dev) {
> +			NL_SET_ERR_MSG(extack, "bad ifindex");
> +			return -ENODEV;
> +		}
> +		*mdev = mctp_dev_get_rtnl(dev);
> +		if (!*mdev)
> +			return -ENODEV;
> +		gatewayp->eid = 0;
> +
> +	} else if (gateway) {

... and here.

Flagged by Smatch.

> +		if (!mctp_address_unicast(gateway->eid)) {
> +			NL_SET_ERR_MSG(extack, "bad gateway");
> +			return -EINVAL;
> +		}
> +
> +		gatewayp->eid = gateway->eid;
> +		gatewayp->net = gateway->net != MCTP_NET_ANY ?
> +			gateway->net :
> +			READ_ONCE(net->mctp.default_net);
> +		*mdev = NULL;
> +
> +	} else {
> +		NL_SET_ERR_MSG(extack, "no route output provided");
>  		return -EINVAL;
>  	}
> -	ifindex = nla_get_u32(tb[RTA_OIF]);
>  
>  	*rtm = nlmsg_data(nlh);
>  	if ((*rtm)->rtm_family != AF_MCTP) {

...