[SLOF] [PATCH 1/3] ipv6: Do not use unitialized MAC address array

[Nikunj A Dadhania]

Thomas Huth <thuth at redhat.com> writes:

> The code in send_ipv6() currently basically looks like this:
>
> 	uint8_t *mac_addr, mac[6];
> 	mac_addr = mac;
> 	...
> 	n = find_neighbor (&ip_dst);
> 	if (ip6_is_multicast (&ip_dst)) {
> 		mac_addr = ip6_to_multicast_mac (&ip_dst, mac);
> 	}
> 	else {
> 		// Check if the MAC address is already cached
> 		if (n) {
> 			if (memcmp(n->mac, null_mac, ETH_ALEN) != 0)
> 				memcpy (mac_addr, &(n->mac), ETH_ALEN);
> 			/* XXX */
> 		}
> 		...
> 	}
> 	...
> 	fill_ethhdr (n->eth_frame, htons(ETHERTYPE_IPv6), get_mac_address(),
> 		     mac_addr);
>
> That means mac_addr initially points to the uninitialized mac[6]
> array on the stack. Now if there was already an entry in the neighbor
> cache, but the MAC address has not been determined yet, that
> uninitialized array could be used as final MAC address for fill_ethhdr()
> (since there is no "else" path at the spot marked with XXX above),
> resulting in random data in the MAC address field of the ethernet packet.
>
> Let's fix this issue by letting mac_addr point to the null_mac by
> default instead, so that it never points to invalid data. Also
> rename mac[6] to mc_mac[6] to make it clear that this array is
> only used for storing the multicast mac address.
>
> Signed-off-by: Thomas Huth <thuth at redhat.com>

Reviewed-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>


> ---
>  clients/net-snk/app/netlib/ipv6.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/clients/net-snk/app/netlib/ipv6.c b/clients/net-snk/app/netlib/ipv6.c
> index 348e79d..5a307ca 100644
> --- a/clients/net-snk/app/netlib/ipv6.c
> +++ b/clients/net-snk/app/netlib/ipv6.c
> @@ -491,9 +491,9 @@ int send_ipv6(int fd, void* buffer, int len)
>  	struct udphdr *udph;
>  	struct icmp6hdr *icmp6h;
>  	ip6_addr_t ip_dst;
> -	uint8_t *mac_addr, mac[6];
> +	uint8_t *mac_addr, mc_mac[6];
>
> -	mac_addr = mac;
> +	mac_addr = null_mac;
>
>  	ip6h    = (struct ip6hdr *) buffer;
>  	udph   = (struct udphdr *)   ((uint8_t *) ip6h + sizeof (struct ip6hdr));
> @@ -526,13 +526,13 @@ int send_ipv6(int fd, void* buffer, int len)
>
>  	// If address is a multicast address, create a proper mac address
>  	if (ip6_is_multicast (&ip_dst)) {
> -		mac_addr = ip6_to_multicast_mac (&ip_dst, mac);
> +		mac_addr = ip6_to_multicast_mac (&ip_dst, mc_mac);
>  	}
>  	else {
>  		// Check if the MAC address is already cached
>  		if (n) {
>  			if (memcmp(n->mac, null_mac, ETH_ALEN) != 0)
> -				memcpy (mac_addr, &(n->mac), ETH_ALEN); /* found it */
> +				mac_addr = n->mac;		/* found it */
>  		} else if (!is_ip6addr_in_my_net(&ip_dst)) {
>  			struct router *gw;
>  			gw = ipv6_get_default_router(&ip6h->src);
> -- 
> 1.8.3.1