netdev - Re: [PATCH net-next v3 2/3] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <00911a84-c4e3-452e-ab51-1275a43ca4b2@redhat.com>
Date: Thu, 17 Jul 2025 11:23:36 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Jakub Sitnicki <jakub@...udflare.com>, Eric Dumazet
 <edumazet@...gle.com>, "David S. Miller" <davem@...emloft.net>,
 Jakub Kicinski <kuba@...nel.org>, Neal Cardwell <ncardwell@...gle.com>,
 Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: netdev@...r.kernel.org, kernel-team@...udflare.com,
 Lee Valentine <lvalentine@...udflare.com>
Subject: Re: [PATCH net-next v3 2/3] tcp: Consider every port when connecting
 with IP_LOCAL_PORT_RANGE

On 7/14/25 6:03 PM, Jakub Sitnicki wrote:
> Solution
> --------
> 
> If there is no IP address conflict with any socket bound to a given local
> port, then from the protocol's perspective, the port can be safely shared.
> 
> With that in mind, modify the port search during connect(), that is
> __inet_hash_connect, to consider all bind buckets (ports) when looking for
> a local port for egress.
> 
> To achieve this, add an extra walk over bhash2 buckets for the port to
> check for IP conflicts. The additional walk is not free, so perform it only
> once per port - during the second phase of conflict checking, when the
> bhash bucket is locked.
> 
> We enable this changed behavior only if the IP_LOCAL_PORT_RANGE socket
> option is set. The rationale is that users are likely to care about using
> every possible local port only when they have deliberately constrained the
> ephemeral port range.

I'm not a big fan of piggybacking additional semantic on existing
socketopt, have you considered a new one?

At very least you will need to update the man page.

> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index d3ce6d0a514e..9d8a9c7c8274 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -1005,6 +1005,52 @@ EXPORT_IPV6_MOD(inet_bhash2_reset_saddr);
>  #define INET_TABLE_PERTURB_SIZE (1 << CONFIG_INET_TABLE_PERTURB_ORDER)
>  static u32 *table_perturb;
>  
> +/* True on source address conflict with another socket. False otherwise. */
> +static inline bool check_bind2_bucket(const struct sock *sk,
> +				      const struct inet_bind2_bucket *tb2)

Please no inline function in c files.

Thanks,

Paolo