Patchwork ipv6: Keep index within tab_unreach[]

login
register
mail settings
Submitter roel kluin
Date June 16, 2009, 6:40 p.m.
Message ID <4A37E715.4060504@gmail.com>
Download mbox | patch
Permalink /patch/28735/
State Accepted
Delegated to: David Miller
Headers show

Comments

roel kluin - June 16, 2009, 6:40 p.m.
Ensure that index `code' remains within array tab_unreach[]

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - June 18, 2009, 2:03 a.m.
From: Roel Kluin <roel.kluin@gmail.com>
Date: Tue, 16 Jun 2009 20:40:21 +0200

> Ensure that index `code' remains within array tab_unreach[]
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Brian Haley - June 18, 2009, 2:10 a.m.
Roel Kluin wrote:
> Ensure that index `code' remains within array tab_unreach[]
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index 36dff88..8f850de 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -923,7 +923,7 @@ int icmpv6_err_convert(int type, int code, int *err)
>  	switch (type) {
>  	case ICMPV6_DEST_UNREACH:
>  		fatal = 1;
> -		if (code <= ICMPV6_PORT_UNREACH) {
> +		if (code <= ICMPV6_PORT_UNREACH && code >= 0) {
>  			*err  = tab_unreach[code].err;
>  			fatal = tab_unreach[code].fatal;
>  		}

The code value in the ICMPv6 header is a u8, so should always be positive, right?
It doesn't hurt I guess though.

-Brian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - June 18, 2009, 2:20 a.m.
From: Brian Haley <brian.haley@hp.com>
Date: Wed, 17 Jun 2009 22:10:08 -0400

> Roel Kluin wrote:
>> @@ -923,7 +923,7 @@ int icmpv6_err_convert(int type, int code, int *err)
>>  	switch (type) {
>>  	case ICMPV6_DEST_UNREACH:
>>  		fatal = 1;
>> -		if (code <= ICMPV6_PORT_UNREACH) {
>> +		if (code <= ICMPV6_PORT_UNREACH && code >= 0) {
>>  			*err  = tab_unreach[code].err;
>>  			fatal = tab_unreach[code].fatal;
>>  		}
> 
> The code value in the ICMPv6 header is a u8, so should always be positive, right?
> It doesn't hurt I guess though.

True.  Probably best to pass this thing in as a 'u8', rathern than as
an 'int'.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Brian Haley - June 18, 2009, 4:09 a.m.
David Miller wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Wed, 17 Jun 2009 22:10:08 -0400
> 
>> Roel Kluin wrote:
>>> @@ -923,7 +923,7 @@ int icmpv6_err_convert(int type, int code, int *err)
>>>  	switch (type) {
>>>  	case ICMPV6_DEST_UNREACH:
>>>  		fatal = 1;
>>> -		if (code <= ICMPV6_PORT_UNREACH) {
>>> +		if (code <= ICMPV6_PORT_UNREACH && code >= 0) {
>>>  			*err  = tab_unreach[code].err;
>>>  			fatal = tab_unreach[code].fatal;
>>>  		}
>> The code value in the ICMPv6 header is a u8, so should always be positive, right?
>> It doesn't hurt I guess though.
> 
> True.  Probably best to pass this thing in as a 'u8', rathern than as
> an 'int'.

"type" is a u8 too, but both seem to be passed as int's everywhere (rawv6,
tcp, sctp and dccp), and in the err_handler() func pointer in the inet6_protocol
struct.  I can try and cook something up in the next few days that changes
it all to u8's.

-Brian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - June 18, 2009, 7:26 a.m.
From: Brian Haley <brian.haley@hp.com>
Date: Thu, 18 Jun 2009 00:09:36 -0400

> "type" is a u8 too, but both seem to be passed as int's everywhere (rawv6,
> tcp, sctp and dccp), and in the err_handler() func pointer in the inet6_protocol
> struct.  I can try and cook something up in the next few days that changes
> it all to u8's.

Feel free, thanks a lot.

I'm going to revert Roel's patch since it doesn't fix anything.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu - July 5, 2009, 3:46 a.m.
Roel Kluin <roel.kluin@gmail.com> wrote:
> Ensure that index `code' remains within array tab_unreach[]
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index 36dff88..8f850de 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -923,7 +923,7 @@ int icmpv6_err_convert(int type, int code, int *err)
>        switch (type) {
>        case ICMPV6_DEST_UNREACH:
>                fatal = 1;
> -               if (code <= ICMPV6_PORT_UNREACH) {
> +               if (code <= ICMPV6_PORT_UNREACH && code >= 0) {

Why not make code unsigned?

Cheers,
David Miller - July 5, 2009, 10:52 p.m.
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sun, 5 Jul 2009 11:46:55 +0800

> Roel Kluin <roel.kluin@gmail.com> wrote:
>> Ensure that index `code' remains within array tab_unreach[]
>> 
>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>> ---
>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>> index 36dff88..8f850de 100644
>> --- a/net/ipv6/icmp.c
>> +++ b/net/ipv6/icmp.c
>> @@ -923,7 +923,7 @@ int icmpv6_err_convert(int type, int code, int *err)
>>        switch (type) {
>>        case ICMPV6_DEST_UNREACH:
>>                fatal = 1;
>> -               if (code <= ICMPV6_PORT_UNREACH) {
>> +               if (code <= ICMPV6_PORT_UNREACH && code >= 0) {
> 
> Why not make code unsigned?

type and code should both be "u8"'s, we made a similar
conversio of a family of ipv6 function arguments recently.

Actually, it's already there and done to this very function.

Herbert, this is just an ancient patch that has been superceded
by the very change you are suggestions :-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu - July 6, 2009, 1:03 a.m.
On Sun, Jul 05, 2009 at 03:52:15PM -0700, David Miller wrote:
>
> type and code should both be "u8"'s, we made a similar
> conversio of a family of ipv6 function arguments recently.
> 
> Actually, it's already there and done to this very function.
> 
> Herbert, this is just an ancient patch that has been superceded
> by the very change you are suggestions :-)

Yeah I know, was just reading old emails backwards :)

Patch

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 36dff88..8f850de 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -923,7 +923,7 @@  int icmpv6_err_convert(int type, int code, int *err)
 	switch (type) {
 	case ICMPV6_DEST_UNREACH:
 		fatal = 1;
-		if (code <= ICMPV6_PORT_UNREACH) {
+		if (code <= ICMPV6_PORT_UNREACH && code >= 0) {
 			*err  = tab_unreach[code].err;
 			fatal = tab_unreach[code].fatal;
 		}