diff mbox

[2/5] ipv6: Clear memory after malloc if necessary

Message ID 1462218931-4573-3-git-send-email-thuth@redhat.com
State Accepted
Headers show

Commit Message

Thomas Huth May 2, 2016, 7:55 p.m. UTC
The IPv6 code uses malloc in a couple of places to allocate the memory
for a struct. But it does not properly initializes all members of the
struct after the allocation, so the uninitialized members might contain
random data. So we should better clear the whole memory for those
structs to make sure we do not run into some hard-to-reproduce random
problems later.

Reported-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 clients/net-snk/app/netlib/ipv6.c | 14 +++++++++++---
 clients/net-snk/app/netlib/ndp.c  |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

Comments

Andrew Jones May 3, 2016, 5:19 a.m. UTC | #1
On Mon, May 02, 2016 at 09:55:28PM +0200, Thomas Huth wrote:
> The IPv6 code uses malloc in a couple of places to allocate the memory
> for a struct. But it does not properly initializes all members of the
> struct after the allocation, so the uninitialized members might contain
> random data. So we should better clear the whole memory for those
> structs to make sure we do not run into some hard-to-reproduce random
> problems later.
> 
> Reported-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  clients/net-snk/app/netlib/ipv6.c | 14 +++++++++++---
>  clients/net-snk/app/netlib/ndp.c  |  1 +
>  2 files changed, 12 insertions(+), 3 deletions(-)

I only reported the one I saw. I see you found many more. I wonder
if it'd be helpful for SLOF to add calloc?

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/clients/net-snk/app/netlib/ipv6.c b/clients/net-snk/app/netlib/ipv6.c
> index 6c041d6..baa5034 100644
> --- a/clients/net-snk/app/netlib/ipv6.c
> +++ b/clients/net-snk/app/netlib/ipv6.c
> @@ -68,7 +68,11 @@ void set_ipv6_address(int fd, ip6_addr_t *_own_ip6)
>  {
>  	struct ip6addr_list_entry *ile;
>  
> -	own_ip6 = malloc (sizeof(struct ip6addr_list_entry));
> +	ile = malloc(sizeof(struct ip6addr_list_entry));
> +	if (!ile)
> +		return;
> +	memset(ile, 0, sizeof(struct ip6addr_list_entry));
> +	own_ip6 = ile;
>  
>  	/* If no address was passed as a parameter generate a link-local
>  	 * address from our MAC address.*/
> @@ -263,6 +267,7 @@ struct prefix_info *ip6_create_prefix_info()
>  	prfx_info = malloc (sizeof(struct prefix_info));
>  	if (!prfx_info)
>  		return NULL;
> +	memset(prfx_info, 0, sizeof(struct prefix_info));
>  
>  	return prfx_info;
>  }
> @@ -282,6 +287,7 @@ void *ip6_prefix2addr(ip6_addr_t prefix)
>  	new_address = malloc (sizeof(struct ip6addr_list_entry));
>  	if( !new_address )
>  		return NULL;
> +	memset(new_address, 0, sizeof(struct ip6addr_list_entry));
>  
>  	/* fill new addr struct */
>  	/* extract prefix from Router Advertisement */
> @@ -317,11 +323,10 @@ int8_t ip6addr_add(struct ip6addr_list_entry *new_address)
>  	 * for its solicited-node multicast address.
>  	 * See RFC 2373 - IP Version 6 Adressing Architecture */
>  	if (! ip6_is_multicast(&(new_address->addr))) {
> -
> -
>  		solicited_node = malloc(sizeof(struct ip6addr_list_entry));
>  		if (! solicited_node)
>  			return 0;
> +		memset(solicited_node, 0, sizeof(struct ip6addr_list_entry));
>  
>  		solicited_node->addr.part.prefix       = IPV6_SOLIC_NODE_PREFIX;
>  		solicited_node->addr.part.interface_id = IPV6_SOLIC_NODE_IFACE_ID;
> @@ -541,6 +546,9 @@ int send_ipv6(int fd, void* buffer, int len)
>  		} else {
>  			mac_addr = null_mac;
>  			n = malloc(sizeof(struct neighbor));
> +			if (!n)
> +				return -1;
> +			memset(n, 0, sizeof(struct neighbor));
>  			memcpy(&(n->ip.addr[0]), &ip_dst, 16);
>  			n->status = NB_PROBE;
>  			n->times_asked += 1;
> diff --git a/clients/net-snk/app/netlib/ndp.c b/clients/net-snk/app/netlib/ndp.c
> index 7a8dfda..263bee2 100644
> --- a/clients/net-snk/app/netlib/ndp.c
> +++ b/clients/net-snk/app/netlib/ndp.c
> @@ -140,6 +140,7 @@ neighbor_create (uint8_t *packet, struct packeth *headers)
>  	new_neighbor = malloc (sizeof(struct neighbor));
>  	if( !new_neighbor )
>  		return NULL;
> +	memset(new_neighbor, 0, sizeof(struct neighbor));
>  
>  	/* fill neighbor struct */
>  	memcpy (&(new_neighbor->mac),
> -- 
> 1.8.3.1
>
diff mbox

Patch

diff --git a/clients/net-snk/app/netlib/ipv6.c b/clients/net-snk/app/netlib/ipv6.c
index 6c041d6..baa5034 100644
--- a/clients/net-snk/app/netlib/ipv6.c
+++ b/clients/net-snk/app/netlib/ipv6.c
@@ -68,7 +68,11 @@  void set_ipv6_address(int fd, ip6_addr_t *_own_ip6)
 {
 	struct ip6addr_list_entry *ile;
 
-	own_ip6 = malloc (sizeof(struct ip6addr_list_entry));
+	ile = malloc(sizeof(struct ip6addr_list_entry));
+	if (!ile)
+		return;
+	memset(ile, 0, sizeof(struct ip6addr_list_entry));
+	own_ip6 = ile;
 
 	/* If no address was passed as a parameter generate a link-local
 	 * address from our MAC address.*/
@@ -263,6 +267,7 @@  struct prefix_info *ip6_create_prefix_info()
 	prfx_info = malloc (sizeof(struct prefix_info));
 	if (!prfx_info)
 		return NULL;
+	memset(prfx_info, 0, sizeof(struct prefix_info));
 
 	return prfx_info;
 }
@@ -282,6 +287,7 @@  void *ip6_prefix2addr(ip6_addr_t prefix)
 	new_address = malloc (sizeof(struct ip6addr_list_entry));
 	if( !new_address )
 		return NULL;
+	memset(new_address, 0, sizeof(struct ip6addr_list_entry));
 
 	/* fill new addr struct */
 	/* extract prefix from Router Advertisement */
@@ -317,11 +323,10 @@  int8_t ip6addr_add(struct ip6addr_list_entry *new_address)
 	 * for its solicited-node multicast address.
 	 * See RFC 2373 - IP Version 6 Adressing Architecture */
 	if (! ip6_is_multicast(&(new_address->addr))) {
-
-
 		solicited_node = malloc(sizeof(struct ip6addr_list_entry));
 		if (! solicited_node)
 			return 0;
+		memset(solicited_node, 0, sizeof(struct ip6addr_list_entry));
 
 		solicited_node->addr.part.prefix       = IPV6_SOLIC_NODE_PREFIX;
 		solicited_node->addr.part.interface_id = IPV6_SOLIC_NODE_IFACE_ID;
@@ -541,6 +546,9 @@  int send_ipv6(int fd, void* buffer, int len)
 		} else {
 			mac_addr = null_mac;
 			n = malloc(sizeof(struct neighbor));
+			if (!n)
+				return -1;
+			memset(n, 0, sizeof(struct neighbor));
 			memcpy(&(n->ip.addr[0]), &ip_dst, 16);
 			n->status = NB_PROBE;
 			n->times_asked += 1;
diff --git a/clients/net-snk/app/netlib/ndp.c b/clients/net-snk/app/netlib/ndp.c
index 7a8dfda..263bee2 100644
--- a/clients/net-snk/app/netlib/ndp.c
+++ b/clients/net-snk/app/netlib/ndp.c
@@ -140,6 +140,7 @@  neighbor_create (uint8_t *packet, struct packeth *headers)
 	new_neighbor = malloc (sizeof(struct neighbor));
 	if( !new_neighbor )
 		return NULL;
+	memset(new_neighbor, 0, sizeof(struct neighbor));
 
 	/* fill neighbor struct */
 	memcpy (&(new_neighbor->mac),