[PATCH] ipv4: kernel panic when only one unsecured port available

[PATCH] ipv4: kernel panic when only one unsecured port available

[Anton Arapov]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

Steps to reproduce:
Server:
  [root@server ~]# cat /etc/exports
  /export *(ro,insecure)
// there is insecure ... I am using ports like "1024 to 61000"
  [root@server ~] service nfs restart 

Client:
  1.[root@client ~]# echo 32768 32768 > /proc/sys/net/ipv4/ip_local_port_range
  32768   32768
// two same numbers, for ex "32769 32769" etc.
  2.[root@client ~]# cat /proc/sys/net/ipv4/ip_local_port_range
  32768   32768
  3.[root@client ~]# mount server:/export /import
  
Actual results:
  Kernel always panics 

--------------------------------------------------------------------
[PATCH] ipv4: kernel panic when only one unsecured port available

  Patch prevents division by zero. Kernel panics if only one 
unsecured port available.

Signed-off-by: Anton Arapov <aarapov@redhat.com>
---

 net/ipv4/inet_connection_sock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index fbe7714..00ad079 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -80,7 +80,7 @@ int inet_csk_get_port(struct inet_hashinfo *hashinfo,
 		int low = sysctl_local_port_range[0];
 		int high = sysctl_local_port_range[1];
 		int remaining = (high - low) + 1;
-		int rover = net_random() % (high - low) + low;
+		int rover = net_random() % remaining + low;
 
 		do {
 			head = &hashinfo->bhash[inet_bhashfn(rover, hashinfo->bhash_size)];

-- 
Anton Arapov, <aarapov@redhat.com>
Kernel Development, Red Hat
GPG Key ID: 0x6FA8C812

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

[PATCH] ipv4: kernel panic when only one unsecured port available

[Anton Arapov]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

Steps to reproduce:
Server:
  [root@server ~]# cat /etc/exports
  /export *(ro,insecure)
// there is insecure ... I am using ports like "1024 to 61000"
  [root@server ~] service nfs restart 

Client:
  1.[root@client ~]# echo 32768 32768 > /proc/sys/net/ipv4/ip_local_port_range
  32768   32768
// two same numbers, for ex "32769 32769" etc.
  2.[root@client ~]# cat /proc/sys/net/ipv4/ip_local_port_range
  32768   32768
  3.[root@client ~]# mount server:/export /import
  
Actual results:
  Kernel always panics 

--------------------------------------------------------------------
[PATCH] ipv4: kernel panic when only one unsecured port available

  Patch prevents division by zero. Kernel panics if only one 
unsecured port available.

Signed-off-by: Anton Arapov <aarapov@redhat.com>
---

 net/ipv4/inet_connection_sock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index fbe7714..00ad079 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -80,7 +80,7 @@ int inet_csk_get_port(struct inet_hashinfo *hashinfo,
 		int low = sysctl_local_port_range[0];
 		int high = sysctl_local_port_range[1];
 		int remaining = (high - low) + 1;
-		int rover = net_random() % (high - low) + low;
+		int rover = net_random() % remaining + low;
 
 		do {
 			head = &hashinfo->bhash[inet_bhashfn(rover, hashinfo->bhash_size)];

-- 
Anton Arapov, <aarapov@redhat.com>
Kernel Development, Red Hat
GPG Key ID: 0x6FA8C812

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Denis V. Lunev]

This code is broken from the very beginning.

iris den # cat /proc/sys/net/ipv4/ip_local_port_range
32768   61000
iris den # echo 32768 32 >/proc/sys/net/ipv4/ip_local_port_range
iris den # cat /proc/sys/net/ipv4/ip_local_port_range
32768   32
iris den # echo 32768 61000 >/proc/sys/net/ipv4/ip_local_port_range

Regards,
	Den

Anton Arapov wrote:
> Steps to reproduce:
> Server:
>   [root@server ~]# cat /etc/exports
>   /export *(ro,insecure)
> // there is insecure ... I am using ports like "1024 to 61000"
>   [root@server ~] service nfs restart 
> 
> Client:
>   1.[root@client ~]# echo 32768 32768 > /proc/sys/net/ipv4/ip_local_port_range
>   32768   32768
> // two same numbers, for ex "32769 32769" etc.
>   2.[root@client ~]# cat /proc/sys/net/ipv4/ip_local_port_range
>   32768   32768
>   3.[root@client ~]# mount server:/export /import
>   
> Actual results:
>   Kernel always panics 
> 
> --------------------------------------------------------------------
> [PATCH] ipv4: kernel panic when only one unsecured port available
> 
>   Patch prevents division by zero. Kernel panics if only one 
> unsecured port available.
> 
> Signed-off-by: Anton Arapov <aarapov@redhat.com>
> ---
> 
>  net/ipv4/inet_connection_sock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index fbe7714..00ad079 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -80,7 +80,7 @@ int inet_csk_get_port(struct inet_hashinfo *hashinfo,
>  		int low = sysctl_local_port_range[0];
>  		int high = sysctl_local_port_range[1];
>  		int remaining = (high - low) + 1;
> -		int rover = net_random() % (high - low) + low;
> +		int rover = net_random() % remaining + low;
>  
>  		do {
>  			head = &hashinfo->bhash[inet_bhashfn(rover, hashinfo->bhash_size)];
> 

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Anton Arapov]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Hi,

"Denis V. Lunev" <den@sw.ru> writes:
> This code is broken from the very beginning.
>
> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
> 32768   61000
> iris den # echo 32768 32 >/proc/sys/net/ipv4/ip_local_port_range
> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
> 32768   32
> iris den # echo 32768 61000 >/proc/sys/net/ipv4/ip_local_port_range

  If you're talking about checks in sysctl, I believe it should be
another patch for sysctl only, and I'm going to push it via -mm tree.

  the devision by zero exists in inet_connection_socket.c, and must be
fixed for sure because the situation with the same min and max port
numbers in sysctl are possible and not prohibited.

Cheers!
-- 
Anton Arapov, <aarapov@redhat.com>
GPG Key ID: 0x6FA8C812

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Denis V. Lunev]

Anton Arapov wrote:
> Hi,
> 
> "Denis V. Lunev" <den@sw.ru> writes:
>> This code is broken from the very beginning.
>>
>> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
>> 32768   61000
>> iris den # echo 32768 32 >/proc/sys/net/ipv4/ip_local_port_range
>> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
>> 32768   32
>> iris den # echo 32768 61000 >/proc/sys/net/ipv4/ip_local_port_range
> 
>   If you're talking about checks in sysctl, I believe it should be
> another patch for sysctl only, and I'm going to push it via -mm tree.
> 
>   the devision by zero exists in inet_connection_socket.c, and must be
> fixed for sure because the situation with the same min and max port
> numbers in sysctl are possible and not prohibited.
> 
> Cheers!

your patch change nothing :( unfortunately. If I set '32768 32767' it
will oops again.

Regards,
	Den

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Anton Arapov]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

"Denis V. Lunev" <den@sw.ru> writes:
> Anton Arapov wrote: 
>> "Denis V. Lunev" <den@sw.ru> writes:
>>> This code is broken from the very beginning.
>>>
>>> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
>>> 32768   61000
>>> iris den # echo 32768 32 >/proc/sys/net/ipv4/ip_local_port_range
>>> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
>>> 32768   32
>>> iris den # echo 32768 61000 >/proc/sys/net/ipv4/ip_local_port_range
>> 
>>   If you're talking about checks in sysctl, I believe it should be
>> another patch for sysctl only, and I'm going to push it via -mm tree.
>> 
>>   the devision by zero exists in inet_connection_socket.c, and must be
>> fixed for sure because the situation with the same min and max port
>> numbers in sysctl are possible and not prohibited.
>> 
>> Cheers!
>
> your patch change nothing :( unfortunately. If I set '32768 32767' it
> will oops again.

  Patch prevents the system crash. System traps on division by zero.

  Your case(MAX<MIN) must be handled by sysctl. Do you want to say
that I have to join patch for sysctl.c to this one? It's bad idea.

-- 
Anton Arapov, <aarapov@redhat.com>
Kernel Development, Red Hat
GPG Key ID: 0x6FA8C812

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Denis V. Lunev]

Anton Arapov wrote:
> "Denis V. Lunev" <den@sw.ru> writes:
>> Anton Arapov wrote: 
>>> "Denis V. Lunev" <den@sw.ru> writes:
>>>> This code is broken from the very beginning.
>>>>
>>>> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
>>>> 32768   61000
>>>> iris den # echo 32768 32 >/proc/sys/net/ipv4/ip_local_port_range
>>>> iris den # cat /proc/sys/net/ipv4/ip_local_port_range
>>>> 32768   32
>>>> iris den # echo 32768 61000 >/proc/sys/net/ipv4/ip_local_port_range
>>>   If you're talking about checks in sysctl, I believe it should be
>>> another patch for sysctl only, and I'm going to push it via -mm tree.
>>>
>>>   the devision by zero exists in inet_connection_socket.c, and must be
>>> fixed for sure because the situation with the same min and max port
>>> numbers in sysctl are possible and not prohibited.
>>>
>>> Cheers!
>> your patch change nothing :( unfortunately. If I set '32768 32767' it
>> will oops again.
> 
>   Patch prevents the system crash. System traps on division by zero.
> 
>   Your case(MAX<MIN) must be handled by sysctl. Do you want to say
> that I have to join patch for sysctl.c to this one? It's bad idea.
> 

both versions of settings, your ones and my ones are _useless_ in real
life. So, we do some sanity fixes. Am I right? If so, we must prevent
all versions of OOPS (aka division by zero here).

I'll send my vision in a moment...

Regards,
	Den

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[David Miller]

From: "Denis V. Lunev" <den@sw.ru>
Date: Wed, 10 Oct 2007 12:38:37 +0400

> both versions of settings, your ones and my ones are _useless_ in real
> life. So, we do some sanity fixes. Am I right? If so, we must prevent
> all versions of OOPS (aka division by zero here).
> 
> I'll send my vision in a moment...

I agree with Denis that we should plug all of the holes when fixing
this.

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Andrew Morton]

On Tue, 09 Oct 2007 15:59:14 +0200
Anton Arapov <aarapov@redhat.com> wrote:

> Steps to reproduce:
> Server:
>   [root@server ~]# cat /etc/exports
>   /export *(ro,insecure)
> // there is insecure ... I am using ports like "1024 to 61000"
>   [root@server ~] service nfs restart 
> 
> Client:
>   1.[root@client ~]# echo 32768 32768 > /proc/sys/net/ipv4/ip_local_port_range
>   32768   32768
> // two same numbers, for ex "32769 32769" etc.
>   2.[root@client ~]# cat /proc/sys/net/ipv4/ip_local_port_range
>   32768   32768
>   3.[root@client ~]# mount server:/export /import
>   
> Actual results:
>   Kernel always panics 
> 
> --------------------------------------------------------------------
> [PATCH] ipv4: kernel panic when only one unsecured port available
> 
>   Patch prevents division by zero. Kernel panics if only one 
> unsecured port available.
> 
> Signed-off-by: Anton Arapov <aarapov@redhat.com>
> ---
> 
>  net/ipv4/inet_connection_sock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index fbe7714..00ad079 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -80,7 +80,7 @@ int inet_csk_get_port(struct inet_hashinfo *hashinfo,
>  		int low = sysctl_local_port_range[0];
>  		int high = sysctl_local_port_range[1];
>  		int remaining = (high - low) + 1;
> -		int rover = net_random() % (high - low) + low;
> +		int rover = net_random() % remaining + low;
>  
>  		do {
>  			head = &hashinfo->bhash[inet_bhashfn(rover, hashinfo->bhash_size)];
> 

This code has recently been reworked, but from my reading, that
divide-by-zero can still occur.  And given that the numbers in
/proc/sys/net/ipv4/ip_local_port_range are inclusive, the arithmetic in
inet_csk_get_port() seems to just be wrong?

So we have this, against David's current devel tree:

--- a/net/ipv4/inet_connection_sock.c~ipv4-kernel-panic-when-only-one-unsecured-port-available
+++ a/net/ipv4/inet_connection_sock.c
@@ -93,7 +93,7 @@ int inet_csk_get_port(struct inet_hashin
 		int remaining, rover, low, high;
 
 		inet_get_local_port_range(&low, &high);
-		remaining = high - low;
+		remaining = high - low + 1;
 		rover = net_random() % remaining + low;
 
 		do {
_


btw, mime-mangled gpg-fancified emails are rather a pain to handle at the
receivnig end.  Good old text/plain works better, please.

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 15 Oct 2007 12:49:19 -0700

> This code has recently been reworked, but from my reading, that
> divide-by-zero can still occur.  And given that the numbers in
> /proc/sys/net/ipv4/ip_local_port_range are inclusive, the arithmetic in
> inet_csk_get_port() seems to just be wrong?
> 
> So we have this, against David's current devel tree:

I'm pretty sure we took care of this, but maybe not :-)

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Andrew Morton]

On Mon, 15 Oct 2007 13:06:14 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Mon, 15 Oct 2007 12:49:19 -0700
> 
> > This code has recently been reworked, but from my reading, that
> > divide-by-zero can still occur.  And given that the numbers in
> > /proc/sys/net/ipv4/ip_local_port_range are inclusive, the arithmetic in
> > inet_csk_get_port() seems to just be wrong?
> > 
> > So we have this, against David's current devel tree:
> 
> I'm pretty sure we took care of this, but maybe not :-)

<looks>

OK, in ipv4_local_port_range() we have 

                if (range[1] <= range[0])
                        ret = -EINVAL;

which will prevent the crashes.

But is it good to disallow high=low?  This disallows a port range of one
single port.  Unless "high" is exclusive.  But
Documentation/filesystems/proc.txt says

: ip_local_port_range
: -------------------
:
: Range of  ports  used  by  TCP  and UDP to choose the local port. Contains two
: numbers, the  first  number  is the lowest port, the second number the highest
: local port.  Default  is  1024-4999.  Should  be  changed  to  32768-61000 for
: high-usage systems.

ie: inclusive.

Documentation/networking/ip-sysctl.txt says

: ip_local_port_range - 2 INTEGERS
: 	Defines the local port range that is used by TCP and UDP to
: 	choose the local port. The first number is the first, the 
: 	second the last local port number. Default value depends on
: 	amount of memory available on the system:
: 	> 128Mb 32768-61000
: 	< 128Mb 1024-4999 or even less.
: 	This number defines number of active connections, which this
: 	system can issue simultaneously to systems not supporting
: 	TCP extensions (timestamps). With tcp_tw_recycle enabled
: 	(i.e. by default) range 1024-4999 is enough to issue up to
: 	2000 connections per second to systems supporting timestamps.

also inclusive.

Re: [PATCH] ipv4: kernel panic when only one unsecured port available

[Anton Arapov]

[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]

Andrew Morton <akpm@linux-foundation.org> writes:
> <looks>
>
> OK, in ipv4_local_port_range() we have 
>
>                 if (range[1] <= range[0])
>                         ret = -EINVAL;
>
[...skipped...]

> : ip_local_port_range
> : -------------------
> :
> : Range of  ports  used  by  TCP  and UDP to choose the local port. Contains two
> : numbers, the  first  number  is the lowest port, the second number the highest
> : local port.  Default  is  1024-4999.  Should  be  changed  to  32768-61000 for
> : high-usage systems.
>
> ie: inclusive.
>
> Documentation/networking/ip-sysctl.txt says
>
> : ip_local_port_range - 2 INTEGERS
> : 	Defines the local port range that is used by TCP and UDP to
> : 	choose the local port. The first number is the first, the 
> : 	second the last local port number. Default value depends on
> : 	amount of memory available on the system:
> : 	> 128Mb 32768-61000
> : 	< 128Mb 1024-4999 or even less.
> : 	This number defines number of active connections, which this
> : 	system can issue simultaneously to systems not supporting
> : 	TCP extensions (timestamps). With tcp_tw_recycle enabled
> : 	(i.e. by default) range 1024-4999 is enough to issue up to
> : 	2000 connections per second to systems supporting timestamps.
>
> also inclusive.

  I'm also agree, that we should have an ability to use the same
minimum/maximum port number for the cases when we want to use only
one port.

-- 
Anton Arapov, <aarapov@redhat.com>
GPG Key ID: 0x6FA8C812

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]