Re: [B.A.T.M.A.N.] [PATCH v3] [alfred] IPv4 multicast distribution support.

[Sven Eckelmann <sven@narfation.org>]

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

Just had a quick look at the ARP cache code while going to lunch (rest was not 
checked):

> +int ipv4_to_mac(struct interface *interface,
> +		const alfred_addr *addr, struct ether_addr *mac)
> +{
> +	ipv4_arp_request(interface, addr, mac);
>  
>  	if (!is_valid_ether_addr(mac->ether_addr_octet))
>  		return -EINVAL;
[...]
> +
> +int ipv4_arp_request(struct interface *interface, const alfred_addr *addr,
> +		     struct ether_addr *mac)
> +{
> +	struct arpreq arpreq;
> +	struct sockaddr_in *sin;
> +
> +	memset(&arpreq, 0, sizeof(arpreq));
> +
> +	sin = (struct sockaddr_in *)&arpreq.arp_pa;
> +	sin->sin_family = AF_INET;
> +	sin->sin_addr.s_addr = addr->ipv4.s_addr;
> +
> +	strcpy(arpreq.arp_dev, interface->interface);
> +	if (ioctl(interface->netsock, SIOCGARP, &arpreq) < 0)
> +		return -1;
> +
> +	if (arpreq.arp_flags & ATF_COM) {
> +		memcpy(mac, arpreq.arp_ha.sa_data, sizeof(*mac));
> +	} else {
> +		perror("arp: incomplete");
> +		return -1;
> +	}
> +
> +	return 0;
> +}


This looks like you are not always initializing/setting the data for mac. And 
you are not checking the return value of ipv4_to_mac. So it is possible that 
your are accessing "random"/"uninitialized" data in is_valid_ether_addr. It 
can happen that your code accept bogus mac addresses (which are valid mac 
addresses but are not actually the mac address of the remote system) - which 
sounds wrong to me.

Kind regards,
	Sven


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]