netdev - Re: [PATCH net-next v2 1/2] tcp: Consider every port when connecting with IP_LOCAL_PORT

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: <874ivmhht7.fsf@cloudflare.com>
Date: Tue, 08 Jul 2025 16:35:00 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: netdev@...r.kernel.org,  Paolo Abeni <pabeni@...hat.com>,  "David S.
 Miller" <davem@...emloft.net>,  Jakub Kicinski <kuba@...nel.org>,  Neal
 Cardwell <ncardwell@...gle.com>,  Kuniyuki Iwashima <kuniyu@...gle.com>,
  kernel-team@...udflare.com,  Lee Valentine <lvalentine@...udflare.com>
Subject: Re: [PATCH net-next v2 1/2] tcp: Consider every port when
 connecting with IP_LOCAL_PORT_RANGE

On Tue, Jul 08, 2025 at 04:38 AM -07, Eric Dumazet wrote:
> On Thu, Jul 3, 2025 at 8:59 AM Jakub Sitnicki <jakub@...udflare.com> wrote:

[...]

>> @@ -1070,6 +1107,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
>>                         if (!inet_bind_bucket_match(tb, net, port, l3mdev))
>>                                 continue;
>>                         if (tb->fastreuse >= 0 || tb->fastreuseport >= 0) {
>> +                               if (unlikely(local_ports))
>> +                                       break; /* optimistic assumption */
>
> I find this quite pessimistic :/
>
> It seems you had some internal code before my recent change (86c2bc293b8130
> "tcp: use RCU lookup in __inet_hash_connect()") ?
>
> Instead, make the RCU changes so that check_bound() can be called from RCU,
> and call it here before taking the decision to break off this loop.

Thanks for taking a look. I appreciate it.

That was intentional. Perhaps a bad call on my side.

My thinking here was that since we're already short on ephemeral ports
when these "blocked buckets" become an issue, then I wouldn't want to
dismiss a potential port in case the socket blocking it from reuse due
to src (IP, port) conflict goes away before we get to the second
(locked) phase of the search.

But then again, in hindsight, if we run into ephemeral port depletion,
then we must be under pressure from outgoing connections, so it seems
like a bad idea to put more stress on the bucket lock.

I will rework it as suggested.

>>                                 rcu_read_unlock();
>>                                 goto next_port;
>>                         }

[...]