Subject: Inappropriate use of bcopy() in ip6_output ?
To: None <tech-net@netbsd.org>
From: Robert Elz <kre@munnari.OZ.AU>
List: tech-net
Date: 09/24/2005 02:21:31
I believe the use of bcopy() in the following code sequence is wrong
(I mean, it should, once, have been ovbcopy(), I don't meant that 
bcopy() should always just be memcpy() these days).

	/*
	 * If there is a routing header, replace destination address field
	 * with the first hop of the routing header.
	 */
	if (exthdrs.ip6e_rthdr) {
		struct ip6_rthdr *rh;
		struct ip6_rthdr0 *rh0;
		struct in6_addr *addr;

		rh = (struct ip6_rthdr *)(mtod(exthdrs.ip6e_rthdr,
		    struct ip6_rthdr *));
		finaldst = ip6->ip6_dst;
		switch (rh->ip6r_type) {
		case IPV6_RTHDR_TYPE_0:
			 rh0 = (struct ip6_rthdr0 *)rh;
			 addr = (struct in6_addr *)(rh0 + 1);
			 ip6->ip6_dst = addr[0];
			 bcopy(&addr[1], &addr[0],
			     sizeof(struct in6_addr) * (rh0->ip6r0_segleft - 1));
			 addr[rh0->ip6r0_segleft - 1] = finaldst;
			 break;
		default:	/* is it possible? */
			 error = EINVAL;
			 goto bad;
		}
	}

That "if" is at line 434 of ip6_output.c version 1.90.  Or just
search for bcopy - this is the first instance of bcopy in ip6_output.c

If rh0->ip6r0_segleft > 2 then that is definitely an overlapping
copy, which means bcopy() performs an undefined operation
(according to bcopy(9) and the way the BSD kernel bcopy() always
functioned).

If ip6r0_segleft == 2, then it is safe.   If ip6r0_segleft == 1,
then no copy happens, and that's safe as well.   segleft cannot
be < 1 when this code is executed.

I think the bcopy() there needs to transform itself into a memmove()
(as we no longer have ovbcopy() to use apparently).

kre

ps: if someone wants a PR filed about this, rather than just fixing it,
assuming there really is something to fix, please file the PR yourself,
I'm about 4 months behind on readong NetBSD related mail and might not
see a reply any time this year!