[PATCH v2] iproute2: lib/utils.c bug fixes

[PATCH v2] iproute2: lib/utils.c bug fixes

[Dash Four]

This patch fixes the following 3 bugs in get_u32/get_u64 functions:

  1. On 32-bit systems, get_u32 could not detect an overflow.
     get_u32(&l, "4294967296", 10) always returned 4294967295
     (ULONG_MAX on 32-bit systems).

  2. get_u64(&ll, "4294967295", 10) was returning an error where
     it shouldn't have (4294967295 is perfectly legitimate value for
     unsigned long long).

  3. get_u64 couldn't detect an overflow errors (arg > ULLONG_MAX)

Changelog:

v2 - Dropped name title from Developer Certificate of Origin

Signed-off-by: Dash Four <mr.dash.four@googlemail.com>
---
  lib/utils.c |    8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index 5bcdbcf..aeee8f1 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -149,10 +149,12 @@ int get_u64(__u64 *val, const char *arg, int base)
  	unsigned long long res;
  	char *ptr;

+	errno = 0;
+
  	if (!arg || !*arg)
  		return -1;
  	res = strtoull(arg, &ptr, base);
-	if (!ptr || ptr == arg || *ptr || res == 0xFFFFFFFFULL)
+	if (!ptr || ptr == arg || *ptr || (res == ULLONG_MAX && errno == ERANGE))
   		return -1;
   	*val = res;
   	return 0;
@@ -163,10 +165,12 @@ int get_u32(__u32 *val, const char *arg, int base)
  	unsigned long res;
  	char *ptr;

+	errno = 0;
+
  	if (!arg || !*arg)
  		return -1;
  	res = strtoul(arg, &ptr, base);
-	if (!ptr || ptr == arg || *ptr || res > 0xFFFFFFFFUL)
+	if (!ptr || ptr == arg || *ptr || res > 0xFFFFFFFFUL || errno == ERANGE)
  		return -1;
  	*val = res;
  	return 0;

Re: [PATCH v2] iproute2: lib/utils.c bug fixes

[Stephen Hemminger]

On Sun, 14 Apr 2013 15:10:54 +0100
Dash Four <mr.dash.four@googlemail.com> wrote:

> This patch fixes the following 3 bugs in get_u32/get_u64 functions:
> 
>   1. On 32-bit systems, get_u32 could not detect an overflow.
>      get_u32(&l, "4294967296", 10) always returned 4294967295
>      (ULONG_MAX on 32-bit systems).
> 
>   2. get_u64(&ll, "4294967295", 10) was returning an error where
>      it shouldn't have (4294967295 is perfectly legitimate value for
>      unsigned long long).
> 
>   3. get_u64 couldn't detect an overflow errors (arg > ULLONG_MAX)
> 
> Changelog:
> 
> v2 - Dropped name title from Developer Certificate of Origin
> 
> Signed-off-by: Dash Four <mr.dash.four@googlemail.com>
> ---
>   lib/utils.c |    8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 

Thanks for your patch. there were lots of related over/underflow errors in
this code which have existed for a long time. I took your patch as motivation
and did multiple fixes to add error checking for all the cases present.

Thank you.

Re: [PATCH v2] iproute2: lib/utils.c bug fixes

[David Miller]

From: Dash Four <mr.dash.four@googlemail.com>
Date: Sun, 14 Apr 2013 15:10:54 +0100

> This patch fixes the following 3 bugs in get_u32/get_u64 functions:
> 
>  1. On 32-bit systems, get_u32 could not detect an overflow.
>     get_u32(&l, "4294967296", 10) always returned 4294967295
>     (ULONG_MAX on 32-bit systems).
> 
>  2. get_u64(&ll, "4294967295", 10) was returning an error where
>     it shouldn't have (4294967295 is perfectly legitimate value for
>     unsigned long long).
> 
>  3. get_u64 couldn't detect an overflow errors (arg > ULLONG_MAX)
> 
> Changelog:
> 
> v2 - Dropped name title from Developer Certificate of Origin
> 
> Signed-off-by: Dash Four <mr.dash.four@googlemail.com>

Stephen already reimplmeneted your fixes in a slightly different
way.

Re: [PATCH v2] iproute2: lib/utils.c bug fixes

[Dash Four]

> Thanks for your patch. there were lots of related over/underflow errors in
> this code which have existed for a long time. I took your patch as motivation
> and did multiple fixes to add error checking for all the cases present.
>   
That's good. Do you have the git url (or any other repository I could 
fetch these from) so that I could apply these changes on our machines 
here? I checked on 
http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git but 
it looks as though the last commit was done 41 hours ago.

> Thank you.
>   
Pleasure!

Re: [PATCH v2] iproute2: lib/utils.c bug fixes

[David Miller]

From: Dash Four <mr.dash.four@googlemail.com>
Date: Sun, 14 Apr 2013 20:34:22 +0100

> 
>> Thanks for your patch. there were lots of related over/underflow
>> errors in
>> this code which have existed for a long time. I took your patch as
>> motivation
>> and did multiple fixes to add error checking for all the cases
>> present.
>>   
> That's good. Do you have the git url (or any other repository I could
> fetch these from) so that I could apply these changes on our machines
> here? I checked on
> http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git
> but it looks as though the last commit was done 41 hours ago.

It's the top commit in his master branch, no special URL is necessary:

http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=697ac63905cb5ca5389cd840462ee9868123b77f

The commit was made two days ago.