netdev - Re: [PATCH net-next v4 03/14] net: mctp: separate routing database from routing operations

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e858532f-3e70-4d97-a088-3218d9822df5@redhat.com>
Date: Tue, 1 Jul 2025 15:14:24 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Jeremy Kerr <jk@...econstruct.com.au>,
 Matt Johnston <matt@...econstruct.com.au>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next v4 03/14] net: mctp: separate routing database
 from routing operations

On 6/27/25 10:52 AM, Jeremy Kerr wrote:
> @@ -133,34 +133,33 @@ static int mctp_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
>  	if (msk->addr_ext && addrlen >= sizeof(struct sockaddr_mctp_ext)) {
>  		DECLARE_SOCKADDR(struct sockaddr_mctp_ext *,
>  				 extaddr, msg->msg_name);
> -		struct net_device *dev;
> -
> -		rc = -EINVAL;
> -		rcu_read_lock();
> -		dev = dev_get_by_index_rcu(sock_net(sk), extaddr->smctp_ifindex);
> -		/* check for correct halen */
> -		if (dev && extaddr->smctp_halen == dev->addr_len) {
> -			hlen = LL_RESERVED_SPACE(dev) + sizeof(struct mctp_hdr);
> -			rc = 0;
> +
> +		if (!mctp_sockaddr_ext_is_ok(extaddr) ||
> +		    extaddr->smctp_halen > sizeof(cb->haddr)) {
> +			rc = -EINVAL;
> +			goto err_free;
>  		}
> -		rcu_read_unlock();
> +
> +		rc = mctp_dst_from_extaddr(&dst, sock_net(sk),
> +					   extaddr->smctp_ifindex,
> +					   extaddr->smctp_halen,
> +					   extaddr->smctp_haddr);
>  		if (rc)
>  			goto err_free;
> -		rt = NULL;
> +
>  	} else {
> -		rt = mctp_route_lookup(sock_net(sk), addr->smctp_network,
> -				       addr->smctp_addr.s_addr);
> -		if (!rt) {
> -			rc = -EHOSTUNREACH;
> +		rc = mctp_route_lookup(sock_net(sk), addr->smctp_network,
> +				       addr->smctp_addr.s_addr, &dst);

AFAICS mctp_route_lookup() intializes 'dst' only when it returns 0...

> @@ -175,30 +174,16 @@ static int mctp_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
>  	cb = __mctp_cb(skb);
>  	cb->net = addr->smctp_network;
>  
> -	if (!rt) {
> -		/* fill extended address in cb */
> -		DECLARE_SOCKADDR(struct sockaddr_mctp_ext *,
> -				 extaddr, msg->msg_name);
> -
> -		if (!mctp_sockaddr_ext_is_ok(extaddr) ||
> -		    extaddr->smctp_halen > sizeof(cb->haddr)) {
> -			rc = -EINVAL;
> -			goto err_free;
> -		}
> -
> -		cb->ifindex = extaddr->smctp_ifindex;
> -		/* smctp_halen is checked above */
> -		cb->halen = extaddr->smctp_halen;
> -		memcpy(cb->haddr, extaddr->smctp_haddr, cb->halen);
> -	}
> -
> -	rc = mctp_local_output(sk, rt, skb, addr->smctp_addr.s_addr,
> +	rc = mctp_local_output(sk, &dst, skb, addr->smctp_addr.s_addr,
>  			       addr->smctp_tag);
>  
> +	mctp_dst_release(&dst);
>  	return rc ? : len;
>  
>  err_free:
>  	kfree_skb(skb);
> +err_release_dst:
> +	mctp_dst_release(&dst);

So here 'dst' should be uninitialized when reaching here via 'goto
err_free' above

/P