[PATCH v5 04/19] net: ipv6: Add Neighbor Discovery Protocol (NDP)

[Daniel Schwierzeck]

On 12/2/22 10:18, Viacheslav Mitrofanov wrote:
> Implement basic of NDP. It doesn't include such things as Router
> Solicitation, Router Advertisement and Redirect. It just has Neighbor
> Solicitation and Neighbor Advertisement. Only these two features are used
> in u-boot IPv6. Implementation of some NDP functions uses API that was
> exposed in "net: ipv6: Add IPv6 basic primitives".
> 
> Also this patch inlcudes update in Makefile to build NDP.
> 
> Series-changes: 3
> - Added structures and functions descriptions
> - Fixed style problems
> 
> Series-changes: 4
> - Fixed structures and functions description style
> 
> Signed-off-by: Viacheslav Mitrofanov <v.v.mitrofanov at yadro.com>
> Reviewed-by: Ramon Fried <rfried.dev at gmail.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
>   include/ndisc.h | 102 +++++++++++++++++
>   net/Makefile    |   1 +
>   net/ndisc.c     | 289 ++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 392 insertions(+)
>   create mode 100644 include/ndisc.h
>   create mode 100644 net/ndisc.c
> 

...

> +
> +int ndisc_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len)
> +{
> +	struct icmp6hdr *icmp =
> +	    (struct icmp6hdr *)(((uchar *)ip6) + IP6_HDR_SIZE);
> +	struct nd_msg *ndisc = (struct nd_msg *)icmp;
> +	uchar neigh_eth_addr[6];
> +
> +	switch (icmp->icmp6_type) {
> +	case IPV6_NDISC_NEIGHBOUR_SOLICITATION:
> +		debug("received neighbor solicitation for %pI6c from %pI6c\n",
> +		      &ndisc->target, &ip6->saddr);
> +		if (ip6_is_our_addr(&ndisc->target) &&
> +		    ndisc_has_option(ip6, ND_OPT_SOURCE_LL_ADDR)) {
> +			ndisc_extract_enetaddr(ndisc, neigh_eth_addr);
> +			ip6_send_na(neigh_eth_addr, &ip6->saddr,
> +				    &ndisc->target);
> +		}
> +		break;
> +
> +	case IPV6_NDISC_NEIGHBOUR_ADVERTISEMENT:
> +		/* are we waiting for a reply ? */
> +		if (ip6_is_unspecified_addr(&net_nd_sol_packet_ip6))
> +			break;
> +
> +		if ((memcmp(&ndisc->target, &net_nd_rep_packet_ip6,
> +			    sizeof(struct in6_addr)) == 0) &&
> +		    ndisc_has_option(ip6, ND_OPT_TARGET_LL_ADDR)) {
> +			ndisc_extract_enetaddr(ndisc, neigh_eth_addr);
> +
> +			/* save address for later use */
> +			if (!net_nd_packet_mac)
> +				memcpy(net_nd_packet_mac, neigh_eth_addr, 7);

Coverity reports the following:

CID 430977:  Null pointer dereferences  (FORWARD_NULL)
Passing null pointer "net_nd_packet_mac" to "memcpy", which dereferences 
it. [Note: The source code implementation of the function has been 
overridden by a builtin model.]

CID 430974:  Memory - corruptions  (OVERRUN)
Overrunning array "neigh_eth_addr" of 6 bytes by passing it to a 
function which accesses it at byte offset 6 using argument "7UL". [Note: 
The source code implementation of the function has been overridden by a 
builtin model.]


Did you mean to write the following which would make more sense?

if (net_nd_packet_mac)
     memcpy(net_nd_packet_mac, neigh_eth_addr, 7);


> +
> +			/* modify header, and transmit it */
> +			memcpy(((struct ethernet_hdr *)net_nd_tx_packet)->et_dest,
> +			       neigh_eth_addr, 6);
> +
> +			net_send_packet(net_nd_tx_packet,
> +					net_nd_tx_packet_size);
> +
> +			/* no ND request pending now */
> +			net_nd_sol_packet_ip6 = net_null_addr_ip6;
> +			net_nd_tx_packet_size = 0;
> +			net_nd_packet_mac = NULL;
> +		}
> +		break;
> +	default:
> +		debug("Unexpected ICMPv6 type 0x%x\n", icmp->icmp6_type);
> +		return -1;
> +	}
> +
> +	return 0;
> +}

-- 
- Daniel