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  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]
Date:   Fri, 23 Dec 2016 13:58:06 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     jbacik@...com
Cc:     hannes@...essinduktion.org, kraigatgoog@...il.com,
        eric.dumazet@...il.com, tom@...bertland.com,
        netdev@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH 4/6 net-next] inet: don't check for bind conflicts
 twice when searching for a port

From: Josef Bacik <jbacik@...com>
Date: Thu, 22 Dec 2016 16:26:36 -0500

> This is just wasted time, we've already found a tb that doesn't have a bind
> conflict, and we don't drop the head lock so scanning again isn't going to give
> us a different answer.  Instead move the tb->reuse setting logic outside of the
> found_tb path and put it in the success: path.  Then make it so that we don't
> goto again if we find a bind conflict in the found_tb path as we won't reach
> this anymore when we are scanning for an ephemeral port.
> 
> Signed-off-by: Josef Bacik <jbacik@...com>
 ...
> @@ -220,8 +219,10 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
>  		spin_lock_bh(&head->lock);
>  		inet_bind_bucket_for_each(tb, &head->chain)
>  			if (net_eq(ib_net(tb), net) && tb->port == port) {
> +				if (hlist_empty(&tb->owners))
> +					goto success;

I am not sure that this condition can ever trigger.

The only time we will see an alive 'tb' with an empty owners list
is when we allocate one in this function.  And when that allocation
succeeds, we go straight through the rest of this function without
ever jumping out for errors or anything like that.

So we should never drop the allocated fresh 'tb', leave it with
an empty owners list, and branch back up to this part of the
function.

Powered by blists - more mailing lists