[PATCH 0/2] IPv6 Network Discovery Boundary Variable and Packed Structure

[PATCH 0/2] IPv6 Network Discovery Boundary Variable and Packed Structure

[emohandesi]

From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>

This series addresses the following.
        1. Coverity Issue (CID 450971): Loop boundary variables should be
	   checked to be within appropriate limits.
        2. Making the structure icmp6_ra_prefix_info packed because it
	   contains network protocol data received from the network.

Ehsan Mohandesi (2):
  net: ipv6: router advertisement message length should be within limits
  net: ipv6: network protocol structures should be packed

 include/net6.h | 2 +-
 net/ndisc.c    | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[PATCH 1/2] net: ipv6: router advertisement message length should be within limits

[emohandesi]

From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>

The argument len passed to function process_ra is the length of the IPv6
router advertisement message and needs to be between 0 and MTU because
it is assigned to remaining_option_len and used as a loop variable.

Addresses-Coverity-ID: 450971 ("TAINTED_SCALAR")
Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
---
 net/ndisc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ndisc.c b/net/ndisc.c
index 0b27779..d1cec06 100644
--- a/net/ndisc.c
+++ b/net/ndisc.c
@@ -382,6 +382,8 @@ int process_ra(struct ip6_hdr *ip6, int len)
 	unsigned char type = 0;
 	struct icmp6_ra_prefix_info *prefix = NULL;
 
+	if (len > ETH_MAX_MTU)
+		return -EMSGSIZE;
 	/* Ignore the packet if router lifetime is 0. */
 	if (!icmp->icmp6_rt_lifetime)
 		return -EOPNOTSUPP;
-- 
1.8.3.1

[PATCH 2/2] net: ipv6: network protocol structures should be packed

[emohandesi]

From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>

The structure icmp6_ra_prefix_info needs to be packed because it is read
from a network stream.

Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
---
 include/net6.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net6.h b/include/net6.h
index beafc05..1e766aa 100644
--- a/include/net6.h
+++ b/include/net6.h
@@ -204,7 +204,7 @@ struct icmp6_ra_prefix_info {
 	 * be initialized to zero by the sender and ignored by the receiver.
 	 */
 	struct in6_addr prefix;
-};
+} __packed;
 
 extern struct in6_addr const net_null_addr_ip6;	/* NULL IPv6 address */
 extern struct in6_addr net_gateway6;	/* Our gateways IPv6 address */
-- 
1.8.3.1

Re: [PATCH 1/2] net: ipv6: router advertisement message length should be within limits

[Vyacheslav V. Mitrofanov]

On Thu, 2023-05-18 at 11:24 -0700, emohandesi@linux.microsoft.com
wrote:
> 
> From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> 
> The argument len passed to function process_ra is the length of the
> IPv6
> router advertisement message and needs to be between 0 and MTU
> because
> it is assigned to remaining_option_len and used as a loop variable.
> 
> Addresses-Coverity-ID: 450971 ("TAINTED_SCALAR")
> Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> ---
>  net/ndisc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/ndisc.c b/net/ndisc.c
> index 0b27779..d1cec06 100644
> --- a/net/ndisc.c
> +++ b/net/ndisc.c
> @@ -382,6 +382,8 @@ int process_ra(struct ip6_hdr *ip6, int len)
>         unsigned char type = 0;
>         struct icmp6_ra_prefix_info *prefix = NULL;
> 
> +       if (len > ETH_MAX_MTU)
> +               return -EMSGSIZE;
>         /* Ignore the packet if router lifetime is 0. */
>         if (!icmp->icmp6_rt_lifetime)
>                 return -EOPNOTSUPP;
> --
> 1.8.3.1
> 
> 
Reviewed-by: Viacheslav Mitrofanov <v.v.mitrofanov@yadro.com>

Re: [PATCH 2/2] net: ipv6: network protocol structures should be packed

[Vyacheslav V. Mitrofanov]

On Thu, 2023-05-18 at 11:24 -0700, emohandesi@linux.microsoft.com
wrote:
> From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> 
> The structure icmp6_ra_prefix_info needs to be packed because it is
> read
> from a network stream.
> 
> Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> ---
>  include/net6.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/net6.h b/include/net6.h
> index beafc05..1e766aa 100644
> --- a/include/net6.h
> +++ b/include/net6.h
> @@ -204,7 +204,7 @@ struct icmp6_ra_prefix_info {
>          * be initialized to zero by the sender and ignored by the
> receiver.
>          */
>         struct in6_addr prefix;
> -};
> +} __packed;
> 
>  extern struct in6_addr const net_null_addr_ip6;        /* NULL IPv6
> address */
>  extern struct in6_addr net_gateway6;   /* Our gateways IPv6 address
> */
> --
> 1.8.3.1
> 
> 
Reviewed-by: Viacheslav Mitrofanov <v.v.mitrofanov@yadro.com>

Re: [PATCH 1/2] net: ipv6: router advertisement message length should be within limits

[Ramon Fried]

On Thu, May 18, 2023 at 9:24 PM <emohandesi@linux.microsoft.com> wrote:
>
> From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
>
> The argument len passed to function process_ra is the length of the IPv6
> router advertisement message and needs to be between 0 and MTU because
> it is assigned to remaining_option_len and used as a loop variable.
>
> Addresses-Coverity-ID: 450971 ("TAINTED_SCALAR")
> Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> ---
>  net/ndisc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/ndisc.c b/net/ndisc.c
> index 0b27779..d1cec06 100644
> --- a/net/ndisc.c
> +++ b/net/ndisc.c
> @@ -382,6 +382,8 @@ int process_ra(struct ip6_hdr *ip6, int len)
>         unsigned char type = 0;
>         struct icmp6_ra_prefix_info *prefix = NULL;
>
> +       if (len > ETH_MAX_MTU)
> +               return -EMSGSIZE;
>         /* Ignore the packet if router lifetime is 0. */
>         if (!icmp->icmp6_rt_lifetime)
>                 return -EOPNOTSUPP;
> --
> 1.8.3.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH 2/2] net: ipv6: network protocol structures should be packed

[Ramon Fried]

On Thu, May 18, 2023 at 9:24 PM <emohandesi@linux.microsoft.com> wrote:
>
> From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
>
> The structure icmp6_ra_prefix_info needs to be packed because it is read
> from a network stream.
>
> Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> ---
>  include/net6.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/net6.h b/include/net6.h
> index beafc05..1e766aa 100644
> --- a/include/net6.h
> +++ b/include/net6.h
> @@ -204,7 +204,7 @@ struct icmp6_ra_prefix_info {
>          * be initialized to zero by the sender and ignored by the receiver.
>          */
>         struct in6_addr prefix;
> -};
> +} __packed;
>
>  extern struct in6_addr const net_null_addr_ip6;        /* NULL IPv6 address */
>  extern struct in6_addr net_gateway6;   /* Our gateways IPv6 address */
> --
> 1.8.3.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH 1/2] net: ipv6: router advertisement message length should be within limits

[Tom Rini]

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

On Thu, May 18, 2023 at 11:24:38AM -0700, emohandesi@linux.microsoft.com wrote:

> From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> 
> The argument len passed to function process_ra is the length of the IPv6
> router advertisement message and needs to be between 0 and MTU because
> it is assigned to remaining_option_len and used as a loop variable.
> 
> Addresses-Coverity-ID: 450971 ("TAINTED_SCALAR")
> Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> Reviewed-by: Viacheslav Mitrofanov <v.v.mitrofanov@yadro.com>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/2] net: ipv6: network protocol structures should be packed

[Tom Rini]

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

On Thu, May 18, 2023 at 11:24:39AM -0700, emohandesi@linux.microsoft.com wrote:

> From: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> 
> The structure icmp6_ra_prefix_info needs to be packed because it is read
> from a network stream.
> 
> Signed-off-by: Ehsan Mohandesi <emohandesi@linux.microsoft.com>
> Reviewed-by: Viacheslav Mitrofanov <v.v.mitrofanov@yadro.com>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]