netdev - Re: [PATCH net-next v11 14/23] ovpn: implement peer lookup logic

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: <ZyivdrpZhx4WpMbn@hog>
Date: Mon, 4 Nov 2024 12:26:46 +0100
From: Sabrina Dubroca <sd@...asysnail.net>
To: Antonio Quartulli <antonio@...nvpn.net>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Donald Hunter <donald.hunter@...il.com>,
	Shuah Khan <shuah@...nel.org>, ryazanov.s.a@...il.com,
	Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v11 14/23] ovpn: implement peer lookup logic

2024-10-29, 11:47:27 +0100, Antonio Quartulli wrote:
>  struct ovpn_peer *ovpn_peer_get_by_transp_addr(struct ovpn_struct *ovpn,
>  					       struct sk_buff *skb)
>  {
> -	struct ovpn_peer *peer = NULL;
> +	struct ovpn_peer *tmp, *peer = NULL;
>  	struct sockaddr_storage ss = { 0 };
> +	struct hlist_nulls_head *nhead;
> +	struct hlist_nulls_node *ntmp;
> +	size_t sa_len;
>  
>  	if (unlikely(!ovpn_peer_skb_to_sockaddr(skb, &ss)))
>  		return NULL;
>  
>  	if (ovpn->mode == OVPN_MODE_P2P)
> -		peer = ovpn_peer_get_by_transp_addr_p2p(ovpn, &ss);
> +		return ovpn_peer_get_by_transp_addr_p2p(ovpn, &ss);
> +
> +	switch (ss.ss_family) {
> +	case AF_INET:
> +		sa_len = sizeof(struct sockaddr_in);
> +		break;
> +	case AF_INET6:
> +		sa_len = sizeof(struct sockaddr_in6);
> +		break;
> +	default:
> +		return NULL;
> +	}

You could get rid of that switch by having ovpn_peer_skb_to_sockaddr
also set sa_len (or return 0/the size).

> +
> +	nhead = ovpn_get_hash_head(ovpn->peers->by_transp_addr, &ss, sa_len);
> +
> +	rcu_read_lock();
> +	hlist_nulls_for_each_entry_rcu(tmp, ntmp, nhead,
> +				       hash_entry_transp_addr) {

I think that's missing the retry in case we ended up in the wrong
bucket due to a peer rehash?

> +		if (!ovpn_peer_transp_match(tmp, &ss))
> +			continue;
> +
> +		if (!ovpn_peer_hold(tmp))
> +			continue;
> +
> +		peer = tmp;
> +		break;
> +	}
> +	rcu_read_unlock();
>  
>  	return peer;
>  }

-- 
Sabrina