net: check net.core.somaxconn sysctl values

PATCH In review Roman Gushchin 3 participants 10 messages Jul 31, 2013 – Aug 3, 2013

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Roman Gushchin

author

2013-07-31 13:57 UTC #0 lore raw

PATCH 1/1 net: check net.core.somaxconn sysctl values

It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.

The sk_max_ack_backlog field of the sock structure is defined as unsigned short. Therefore, the backlog argument in inet_listen() shouldn't exceed USHRT_MAX. The backlog argument in the listen() syscall is truncated to the somaxconn value. So, the somaxconn value shouldn't exceed 65535 (USHRT_MAX).
Also, negative values of somaxconn are meaningless.

before:

$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100

after:

$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"

Signed-off-by Roman Gushchin <[email protected]>

---
 net/core/sysctl_net_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

net/core/sysctl_net_core.c +5 -1

index cfdb46a..2ff093b 100644
@@ -20,7 +20,9 @@
 #include <net/sock.h>
 #include <net/net_ratelimit.h>
 
+static int zero = 0;
 static int one = 1;
+static int ushort_max = USHRT_MAX;
 
 #ifdef CONFIG_RPS
 static int rps_sock_flow_sysctl(ctl_table *table, int write,

@@ -204,7 +206,9 @@ static struct ctl_table netns_core_table[] = {
 		.data		= &init_net.core.sysctl_somaxconn,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec
+		.extra1		= &zero,
+		.extra2		= &ushort_max,
+		.proc_handler	= proc_dointvec_minmax
 	},
 	{ }
 };

Signature · 1 line

1.8.1.2

Eric Dumazet

Acked-by on v1 1/1

2013-07-31 14:37 UTC #1 lore raw

Quoted 26 lines · Roman Gushchin

It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.
The sk_max_ack_backlog field of the sock structure is defined as unsigned short. Therefore, the backlog argument in inet_listen() shouldn't exceed USHRT_MAX. The backlog argument in the listen() syscall is truncated to the somaxconn value. So, the somaxconn value shouldn't exceed 65535 (USHRT_MAX).
Also, negative values of somaxconn are meaningless.
before:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100
after:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"

Quoted 4 lines

Signed-off-by Roman Gushchin <[email protected]>
---
  net/core/sysctl_net_core.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

We had a discussion about this one month ago, as Changli Gao posted a patch. http://patchwork.ozlabs.org/patch/255460/

So proper credits would be nice ;)

Based on a prior patch from Changli Gao

Reported-by Changli Gao <[email protected]>

Acked-by Eric Dumazet <[email protected]>

Roman Gushchin

author Acked-by Eric Dumazet

2013-07-31 14:54 UTC #2 lore raw

FYI: Re: [PATCH] net: check net.core.somaxconn sysctl values

-------- Original Message --------
Subject: Re: [PATCH] net: check net.core.somaxconn sysctl values
Date: Wed, 31 Jul 2013 07:37:37 -0700
From: Eric Dumazet <[email protected]>
To: Roman Gushchin <[email protected]>
CC: David S. Miller <[email protected]>, [email protected], [email protected], [email protected], [email protected]

Quoted 26 lines · Roman Gushchin

It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.
The sk_max_ack_backlog field of the sock structure is defined as unsigned short. Therefore, the backlog argument in inet_listen() shouldn't exceed USHRT_MAX. The backlog argument in the listen() syscall is truncated to the somaxconn value. So, the somaxconn value shouldn't exceed 65535 (USHRT_MAX).
Also, negative values of somaxconn are meaningless.
before:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100
after:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"

Quoted 4 lines

Signed-off-by Roman Gushchin <[email protected]>
---
  net/core/sysctl_net_core.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

We had a discussion about this one month ago, as Changli Gao posted a patch. http://patchwork.ozlabs.org/patch/255460/

So proper credits would be nice ;)

Based on a prior patch from Changli Gao

Reported-by Changli Gao <[email protected]>

Acked-by Eric Dumazet <[email protected]>

Roman Gushchin

author

2013-07-31 15:03 UTC #3 lore raw

In reply to Eric Dumazet

Quoted 33 lines · Eric Dumazet

Quoted 26 lines · Roman Gushchin
It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.
The sk_max_ack_backlog field of the sock structure is defined as unsigned short. Therefore, the backlog argument in inet_listen() shouldn't exceed USHRT_MAX. The backlog argument in the listen() syscall is truncated to the somaxconn value. So, the somaxconn value shouldn't exceed 65535 (USHRT_MAX).
Also, negative values of somaxconn are meaningless.
before:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100
after:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"
Quoted 4 lines
Signed-off-by Roman Gushchin <[email protected]>
---
   net/core/sysctl_net_core.c | 6 +++++-
   1 file changed, 5 insertions(+), 1 deletion(-)

Quoted 4 lines

We had a discussion about this one month ago, as Changli Gao posted a patch. http://patchwork.ozlabs.org/patch/255460/
So proper credits would be nice ;)

Ok :)

Quoted 7 lines

Based on a prior patch from Changli Gao
Reported-by Changli Gao <[email protected]>
Suggested-by Eric Dumazet <[email protected]>
Acked-by Eric Dumazet <[email protected]>

Thanks!

PS I've forwarded your letter back to the lkml by mistake. Sorry)

Regards,
Roman

David Miller

maintainer

2013-08-01 00:10 UTC #4 lore raw

In reply to Roman Gushchin

From: Roman Gushchin <[email protected]>
Date: Wed, 31 Jul 2013 17:57:35 +0400

net/core/sysctl_net_core.c

Quoted 9 lines

---
 net/core/sysctl_net_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

net/core/sysctl_net_core.c +0 -0

index cfdb46a..2ff093b 100644
@@ -20,7 +20,9 @@

This patch is against old sources, please respin it against the current tree.

Thanks.

Roman Gushchin

author Acked-by Eric Dumazet

2013-08-01 09:04 UTC #5 lore raw

Quoted 16 lines · David Miller

From: Roman Gushchin <[email protected]>
Date: Wed, 31 Jul 2013 17:57:35 +0400
net/core/sysctl_net_core.c
Quoted 9 lines
---
  net/core/sysctl_net_core.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
net/core/sysctl_net_core.c +0 -0
index cfdb46a..2ff093b 100644
@@ -20,7 +20,9 @@
This patch is against old sources, please respin it against the current tree.

Quoted 2 lines

Thanks.

net: check net.core.somaxconn sysctl values

It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.

before:

$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100

after:

$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"

Based on a prior patch from Changli Gao.

Signed-off-by Roman Gushchin <[email protected]>

Reported-by Changli Gao <[email protected]>

Acked-by Eric Dumazet <[email protected]>

---
 net/core/sysctl_net_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

net/core/sysctl_net_core.c +5 -1

index 6609686..7c37dcd 100644
@@ -21,7 +21,9 @@
 #include <net/net_ratelimit.h>
 #include <net/busy_poll.h>
 
+static int zero = 0;
 static int one = 1;
+static int ushort_max = USHRT_MAX;
 
 #ifdef CONFIG_RPS
 static int rps_sock_flow_sysctl(struct ctl_table *table, int write,

@@ -339,7 +341,9 @@ static struct ctl_table netns_core_table[] = {
 		.data		= &init_net.core.sysctl_somaxconn,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec
+		.extra1		= &zero,
+		.extra2		= &ushort_max,
+		.proc_handler	= proc_dointvec_minmax
 	},
 	{ }
 };

Signature · 1 line

1.8.1.2

David Miller

maintainer

2013-08-01 21:18 UTC #6 lore raw

From: Roman Gushchin <[email protected]>
Date: Thu, 01 Aug 2013 13:04:16 +0400

Quoted 18 lines · David Miller

Quoted 17 lines
From: Roman Gushchin <[email protected]>
Date: Wed, 31 Jul 2013 17:57:35 +0400
net/core/sysctl_net_core.c
Quoted 9 lines
---
  net/core/sysctl_net_core.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
net/core/sysctl_net_core.c +0 -0
index cfdb46a..2ff093b 100644
@@ -20,7 +20,9 @@
This patch is against old sources, please respin it against the current
tree.

Quoted 4 lines

Quoted 2 lines
Thanks.
net: check net.core.somaxconn sysctl values

Still doesn't apply, and it's because your email client has corrupted the patch.

Please read Documentation/email-clients.txt to learn how to fix this problem, and then send a test patch to yourself.

Only when you can successfully apply the test patch you send to yourself should you repost your patch here.

Thanks.

Roman Gushchin

author

2013-08-02 14:36 UTC #7 lore raw

PATCH 1/1 net: check net.core.somaxconn sysctl values

It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.

before:

$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100

after:

$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"

Based on a prior patch from Changli Gao.

Signed-off-by Roman Gushchin <[email protected]>

Reported-by Changli Gao <[email protected]>

Acked-by Eric Dumazet <[email protected]>

---
 net/core/sysctl_net_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

net/core/sysctl_net_core.c +5 -1

index 6609686..7c37dcd 100644
@@ -21,7 +21,9 @@
 #include <net/net_ratelimit.h>
 #include <net/busy_poll.h>
 
+static int zero = 0;
 static int one = 1;
+static int ushort_max = USHRT_MAX;
 
 #ifdef CONFIG_RPS
 static int rps_sock_flow_sysctl(struct ctl_table *table, int write,

@@ -339,7 +341,9 @@ static struct ctl_table netns_core_table[] = {
 		.data		= &init_net.core.sysctl_somaxconn,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec
+		.extra1		= &zero,
+		.extra2		= &ushort_max,
+		.proc_handler	= proc_dointvec_minmax
 	},
 	{ }
 };

Signature · 1 line

1.8.1.2

David Miller

maintainer Applied

2013-08-02 22:19 UTC #8 lore raw

From: Roman Gushchin <[email protected]>
Date: Fri, 2 Aug 2013 18:36:40 +0400

Quoted 26 lines

It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.
The sk_max_ack_backlog field of the sock structure is defined as unsigned short. Therefore, the backlog argument in inet_listen() shouldn't exceed USHRT_MAX. The backlog argument in the listen() syscall is truncated to the somaxconn value. So, the somaxconn value shouldn't exceed 65535 (USHRT_MAX).
Also, negative values of somaxconn are meaningless.
before:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100
after:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"

Quoted 6 lines

Based on a prior patch from Changli Gao.
Signed-off-by Roman Gushchin <[email protected]>
Reported-by Changli Gao <[email protected]>
Suggested-by Eric Dumazet <[email protected]>
Acked-by Eric Dumazet <[email protected]>

Applied, thanks.

Roman Gushchin

author

2013-08-03 10:14 UTC #9 lore raw

Quoted 29 lines · David Miller

From: Roman Gushchin <[email protected]>
Date: Fri, 2 Aug 2013 18:36:40 +0400
Quoted 26 lines
It's possible to assign an invalid value to the net.core.somaxconn sysctl variable, because there is no checks at all.
The sk_max_ack_backlog field of the sock structure is defined as unsigned short. Therefore, the backlog argument in inet_listen() shouldn't exceed USHRT_MAX. The backlog argument in the listen() syscall is truncated to the somaxconn value. So, the somaxconn value shouldn't exceed 65535 (USHRT_MAX).
Also, negative values of somaxconn are meaningless.
before:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
net.core.somaxconn = 65536
$ sysctl -w net.core.somaxconn=-100
net.core.somaxconn = -100
after:
$ sysctl -w net.core.somaxconn=256
net.core.somaxconn = 256
$ sysctl -w net.core.somaxconn=65536
error: "Invalid argument" setting key "net.core.somaxconn"
$ sysctl -w net.core.somaxconn=-100
error: "Invalid argument" setting key "net.core.somaxconn"

Quoted 8 lines

Quoted 6 lines
Based on a prior patch from Changli Gao.
Signed-off-by Roman Gushchin <[email protected]>
Reported-by Changli Gao <[email protected]>
Suggested-by Eric Dumazet <[email protected]>
Acked-by Eric Dumazet <[email protected]>
Applied, thanks.

Thank you!

Regards,
Roman

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/