All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: document a side effect of ip_local_reserved_ports
@ 2021-04-01 15:57 Otto Hollmann
  2021-04-01 16:04 ` Eric Dumazet
  2021-04-01 23:20 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Otto Hollmann @ 2021-04-01 15:57 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev, linux-kernel,
	Jonathan Corbet, David Ahern
  Cc: Michal Kubecek, Eric Dumazet, Otto Hollmann

    If there is overlapp between ip_local_port_range and ip_local_reserved_ports with a huge reserved block, it will affect probability of selecting ephemeral ports, see file net/ipv4/inet_hashtables.c:723

    int __inet_hash_connect(
    ...
            for (i = 0; i < remaining; i += 2, port += 2) {
                    if (unlikely(port >= high))
                            port -= remaining;
                    if (inet_is_local_reserved_port(net, port))
                            continue;

    E.g. if there is reserved block of 10000 ports, two ports right after this block will be 5000 more likely selected than others.
    If this was intended, we can/should add note into documentation as proposed in this commit, otherwise we should think about different solution. One option could be mapping table of continuous port ranges. Second option could be letting user to modify step (port+=2) in above loop, e.g. using new sysctl parameter.

Signed-off-by: Otto Hollmann <otto.hollmann@suse.com>
---
 Documentation/networking/ip-sysctl.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index c7952ac5bd2f..74c458d2686a 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -1073,7 +1073,9 @@ ip_local_reserved_ports - list of comma separated ranges
 
 	although this is redundant. However such a setting is useful
 	if later the port range is changed to a value that will
-	include the reserved ports.
+	include the reserved ports. Also keep in mind, that overlapping
+	of these ranges may affect probability of selecting ephemeral
+	ports which are right after block of reserved ports.
 
 	Default: Empty
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: document a side effect of ip_local_reserved_ports
  2021-04-01 15:57 [PATCH net-next] net: document a side effect of ip_local_reserved_ports Otto Hollmann
@ 2021-04-01 16:04 ` Eric Dumazet
  2021-04-01 23:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Dumazet @ 2021-04-01 16:04 UTC (permalink / raw)
  To: Otto Hollmann
  Cc: David S. Miller, Jakub Kicinski, netdev, LKML, Jonathan Corbet,
	David Ahern, Michal Kubecek

On Thu, Apr 1, 2021 at 5:58 PM Otto Hollmann <otto.hollmann@suse.com> wrote:
>
>     If there is overlapp between ip_local_port_range and ip_local_reserved_ports with a huge reserved block, it will affect probability of selecting ephemeral ports, see file net/ipv4/inet_hashtables.c:723
>
>     int __inet_hash_connect(
>     ...
>             for (i = 0; i < remaining; i += 2, port += 2) {
>                     if (unlikely(port >= high))
>                             port -= remaining;
>                     if (inet_is_local_reserved_port(net, port))
>                             continue;
>
>     E.g. if there is reserved block of 10000 ports, two ports right after this block will be 5000 more likely selected than others.
>     If this was intended, we can/should add note into documentation as proposed in this commit, otherwise we should think about different solution. One option could be mapping table of continuous port ranges. Second option could be letting user to modify step (port+=2) in above loop, e.g. using new sysctl parameter.
>
> Signed-off-by: Otto Hollmann <otto.hollmann@suse.com>

I think we can view this as a security bug that needs a fix.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] net: document a side effect of ip_local_reserved_ports
  2021-04-01 15:57 [PATCH net-next] net: document a side effect of ip_local_reserved_ports Otto Hollmann
  2021-04-01 16:04 ` Eric Dumazet
@ 2021-04-01 23:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-04-01 23:20 UTC (permalink / raw)
  To: Otto Hollmann
  Cc: davem, kuba, netdev, linux-kernel, corbet, dsahern, mkubecek, edumazet

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Thu,  1 Apr 2021 17:57:05 +0200 you wrote:
> If there is overlapp between ip_local_port_range and ip_local_reserved_ports with a huge reserved block, it will affect probability of selecting ephemeral ports, see file net/ipv4/inet_hashtables.c:723
> 
>     int __inet_hash_connect(
>     ...
>             for (i = 0; i < remaining; i += 2, port += 2) {
>                     if (unlikely(port >= high))
>                             port -= remaining;
>                     if (inet_is_local_reserved_port(net, port))
>                             continue;
> 
> [...]

Here is the summary with links:
  - [net-next] net: document a side effect of ip_local_reserved_ports
    https://git.kernel.org/netdev/net-next/c/a7a80b17c750

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-01 23:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-01 15:57 [PATCH net-next] net: document a side effect of ip_local_reserved_ports Otto Hollmann
2021-04-01 16:04 ` Eric Dumazet
2021-04-01 23:20 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.