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!