diff mbox

[U-Boot,RFC,v2,03/11] lib: net_utils: make string_to_ip stricter

Message ID 1447054736-27658-4-git-send-email-judge.packham@gmail.com
State RFC
Delegated to: Joe Hershberger
Headers show

Commit Message

Chris Packham Nov. 9, 2015, 7:38 a.m. UTC
Previously values greater than 255 were implicitly truncated. Add some
stricter checking to reject addresses with components >255.

With the input "1234192.168.1.1" the old behaviour would truncate the
address to 192.168.1.1. New behaviour rejects the string outright and
returns 0.0.0.0, which for the purposes of IP addresses can be
considered an error.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

Changes in v2:
- restore some lazy parsing behavior that the tftpboot command relied on.

 lib/net_utils.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Joe Hershberger Nov. 24, 2015, 1:06 a.m. UTC | #1
On Mon, Nov 9, 2015 at 1:38 AM, Chris Packham <judge.packham@gmail.com> wrote:
> Previously values greater than 255 were implicitly truncated. Add some
> stricter checking to reject addresses with components >255.
>
> With the input "1234192.168.1.1" the old behaviour would truncate the
> address to 192.168.1.1. New behaviour rejects the string outright and
> returns 0.0.0.0, which for the purposes of IP addresses can be
> considered an error.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
>
> Changes in v2:
> - restore some lazy parsing behavior that the tftpboot command relied on.

It would be good to explicitly describe (in the change log) what you
had to change about your stricter parsing that tftpboot required. It
seems your commit log didn't change.

>  lib/net_utils.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/lib/net_utils.c b/lib/net_utils.c
> index cfae842..f148b8a 100644
> --- a/lib/net_utils.c
> +++ b/lib/net_utils.c
> @@ -24,11 +24,16 @@ struct in_addr string_to_ip(const char *s)
>
>         for (addr.s_addr = 0, i = 0; i < 4; ++i) {
>                 ulong val = s ? simple_strtoul(s, &e, 10) : 0;
> +               if (val > 255) {
> +                       addr.s_addr = 0;
> +                       return addr;
> +               }
>                 addr.s_addr <<= 8;
>                 addr.s_addr |= (val & 0xFF);
> -               if (s) {
> -                       s = (*e) ? e+1 : e;
> -               }
> +               if (*e == '.')
> +                       s = e + 1;
> +               else
> +                       break;
>         }
>
>         addr.s_addr = htonl(addr.s_addr);
> --
> 2.5.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Chris Packham Nov. 24, 2015, 9:37 a.m. UTC | #2
On Tue, Nov 24, 2015 at 2:06 PM, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> On Mon, Nov 9, 2015 at 1:38 AM, Chris Packham <judge.packham@gmail.com> wrote:
>> Previously values greater than 255 were implicitly truncated. Add some
>> stricter checking to reject addresses with components >255.
>>
>> With the input "1234192.168.1.1" the old behaviour would truncate the
>> address to 192.168.1.1. New behaviour rejects the string outright and
>> returns 0.0.0.0, which for the purposes of IP addresses can be
>> considered an error.
>>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>> ---
>>
>> Changes in v2:
>> - restore some lazy parsing behavior that the tftpboot command relied on.
>
> It would be good to explicitly describe (in the change log) what you
> had to change about your stricter parsing that tftpboot required. It
> seems your commit log didn't change.
>

Will do. For the record it the issue was that the tftpboot code
expected to be able parse the ip address part from
'192.168.1.1:zImage'. At some point it might be worth adopting an api
like strtoul which gives you a pointer to where the parsing stopped.

>>  lib/net_utils.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/net_utils.c b/lib/net_utils.c
>> index cfae842..f148b8a 100644
>> --- a/lib/net_utils.c
>> +++ b/lib/net_utils.c
>> @@ -24,11 +24,16 @@ struct in_addr string_to_ip(const char *s)
>>
>>         for (addr.s_addr = 0, i = 0; i < 4; ++i) {
>>                 ulong val = s ? simple_strtoul(s, &e, 10) : 0;
>> +               if (val > 255) {
>> +                       addr.s_addr = 0;
>> +                       return addr;
>> +               }
>>                 addr.s_addr <<= 8;
>>                 addr.s_addr |= (val & 0xFF);
>> -               if (s) {
>> -                       s = (*e) ? e+1 : e;
>> -               }
>> +               if (*e == '.')
>> +                       s = e + 1;
>> +               else
>> +                       break;
>>         }
>>
>>         addr.s_addr = htonl(addr.s_addr);
>> --
>> 2.5.3
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

diff --git a/lib/net_utils.c b/lib/net_utils.c
index cfae842..f148b8a 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -24,11 +24,16 @@  struct in_addr string_to_ip(const char *s)
 
 	for (addr.s_addr = 0, i = 0; i < 4; ++i) {
 		ulong val = s ? simple_strtoul(s, &e, 10) : 0;
+		if (val > 255) {
+			addr.s_addr = 0;
+			return addr;
+		}
 		addr.s_addr <<= 8;
 		addr.s_addr |= (val & 0xFF);
-		if (s) {
-			s = (*e) ? e+1 : e;
-		}
+		if (*e == '.')
+			s = e + 1;
+		else
+			break;
 	}
 
 	addr.s_addr = htonl(addr.s_addr);