netdev - Re: [PATCH net-next v2 1/3] net: Add a bhash2 table hashed by port + address

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2a43287c8ec57119b9dcab46bd6fe7317b9f1f69.camel@redhat.com>
Date:   Thu, 14 Jul 2022 11:08:29 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Joanne Koong <joannelkoong@...il.com>, netdev@...r.kernel.org
Cc:     edumazet@...gle.com, kafai@...com, kuba@...nel.org,
        davem@...emloft.net
Subject: Re: [PATCH net-next v2 1/3] net: Add a bhash2 table hashed by port
 + address

On Tue, 2022-07-12 at 16:53 -0700, Joanne Koong wrote:
> @@ -238,12 +331,23 @@ inet_csk_find_open_port(struct sock *sk, struct inet_bind_bucket **tb_ret, int *
>  			continue;
>  		head = &hinfo->bhash[inet_bhashfn(net, port,
>  						  hinfo->bhash_size)];
> +		head2 = inet_bhashfn_portaddr(hinfo, sk, net, port);
> +
>  		spin_lock_bh(&head->lock);
> +
> +		if (inet_use_bhash2_on_bind(sk)) {
> +			if (inet_bhash2_addr_any_conflict(sk, port, l3mdev, relax, false))
> +				goto next_port;
> +		}
> +
> +		spin_lock(&head2->lock);

Minor nit: it looks like you can compute hash2 but not use it if the
inet_bhash2_addr_any_conflict() call above is unsuccesful. You can move
the inet_bhashfn_portaddr() down. 


[...]

> @@ -675,6 +785,112 @@ void inet_unhash(struct sock *sk)
>  }
>  EXPORT_SYMBOL_GPL(inet_unhash);
>  
> +static bool inet_bind2_bucket_match(const struct inet_bind2_bucket *tb,
> +				    const struct net *net, unsigned short port,
> +				    int l3mdev, const struct sock *sk)
> +{
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (sk->sk_family == AF_INET6)
> +		return net_eq(ib2_net(tb), net) && tb->port == port &&
> +			tb->l3mdev == l3mdev &&
> +			ipv6_addr_equal(&tb->v6_rcv_saddr, &sk->sk_v6_rcv_saddr);
> +	else
> +#endif
> +		return net_eq(ib2_net(tb), net) && tb->port == port &&
> +			tb->l3mdev == l3mdev && tb->rcv_saddr == sk->sk_rcv_saddr;
> +}
> +
> +bool inet_bind2_bucket_match_addr_any(const struct inet_bind2_bucket *tb, const struct net *net,
> +				      unsigned short port, int l3mdev, const struct sock *sk)
> +{
> +#if IS_ENABLED(CONFIG_IPV6)
> +	struct in6_addr addr_any = {};
> +
> +	if (sk->sk_family == AF_INET6)
> +		return net_eq(ib2_net(tb), net) && tb->port == port &&
> +			tb->l3mdev == l3mdev &&
> +			ipv6_addr_equal(&tb->v6_rcv_saddr, &addr_any);
> +	else
> +#endif
> +		return net_eq(ib2_net(tb), net) && tb->port == port &&
> +			tb->l3mdev == l3mdev && tb->rcv_saddr == 0;
> +}
> +
> +/* The socket's bhash2 hashbucket spinlock must be held when this is called */
> +struct inet_bind2_bucket *
> +inet_bind2_bucket_find(const struct inet_bind_hashbucket *head, const struct net *net,
> +		       unsigned short port, int l3mdev, const struct sock *sk)
> +{
> +	struct inet_bind2_bucket *bhash2 = NULL;
> +
> +	inet_bind_bucket_for_each(bhash2, &head->chain)
> +		if (inet_bind2_bucket_match(bhash2, net, port, l3mdev, sk))
> +			break;
> +
> +	return bhash2;
> +}
> +
> +struct inet_bind_hashbucket *
> +inet_bhash2_addr_any_hashbucket(const struct sock *sk, const struct net *net, int port)
> +{
> +	struct inet_hashinfo *hinfo = sk->sk_prot->h.hashinfo;
> +	u32 hash;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	struct in6_addr addr_any = {};
> +
> +	if (sk->sk_family == AF_INET6)
> +		hash = ipv6_portaddr_hash(net, &addr_any, port);
> +	else
> +#endif
> +		hash = ipv4_portaddr_hash(net, 0, port);
> +
> +	return &hinfo->bhash2[hash & (hinfo->bhash_size - 1)];
> +}
> +
> +int inet_bhash2_update_saddr(struct inet_bind_hashbucket *prev_saddr, struct sock *sk)
> +{
> +	struct inet_hashinfo *hinfo = sk->sk_prot->h.hashinfo;
> +	struct inet_bind_hashbucket *head, *head2;
> +	struct inet_bind2_bucket *tb2, *new_tb2;
> +	int l3mdev = inet_sk_bound_l3mdev(sk);
> +	int port = inet_sk(sk)->inet_num;
> +	struct net *net = sock_net(sk);
> +
> +	/* Allocate a bind2 bucket ahead of time to avoid permanently putting
> +	 * the bhash2 table in an inconsistent state if a new tb2 bucket
> +	 * allocation fails.
> +	 */
> +	new_tb2 = kmem_cache_alloc(hinfo->bind2_bucket_cachep, GFP_ATOMIC);
> +	if (!new_tb2)
> +		return -ENOMEM;
> +
> +	head = &hinfo->bhash[inet_bhashfn(net, port,
> +					  hinfo->bhash_size)];

Here 'head' is unused, you can avoid computing the related hash.


Cheers,

Paolo