Skip to contentSkip to conversation

iproute2: lib/utils.c bug fixes

PATCH In review Dash Four 3 participants 5 messages Apr 14, 2013

Subsystems (3)
lore
Share & tools
lore

Dash Four

author
#0 lore raw

PATCH v2 1/1 iproute2: lib/utils.c bug fixes

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

---
 lib/utils.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
lib/utils.c +6 -2
index 5bcdbcf..aeee8f1 100644
@@ -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;
#1 lore raw
Quoted 12 lines · On Sun, 14 Apr 2013 15:10:54 +0100 Dash Four <[email protected]>…

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)

Quoted 9 lines

Changelog:

v2 - Dropped name title from Developer Certificate of Origin

---
  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.

author
#2 lore raw
Quoted 5 lines

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.

Quoted 2 lines

Thank you.

Pleasure!

maintainer
#3 lore raw

From: Dash Four <[email protected]>
Date: Sun, 14 Apr 2013 20:34:22 +0100

Quoted 8 lines
Quoted 8 lines

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.

Quoted 5 lines

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.

maintainer
#4 lore raw
In reply to Dash Four

From: Dash Four <[email protected]>
Date: Sun, 14 Apr 2013 15:10:54 +0100

Quoted 10 lines

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).

Quoted 7 lines

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

Changelog:

v2 - Dropped name title from Developer Certificate of Origin

Stephen already reimplmeneted your fixes in a slightly different way.