Date: Sat, 2 Nov 2013 16:22:12 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Adrian Chadd <adrian@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r257535 - head/sys/netgraph Message-ID: <20131102151309.A1102@besplex.bde.org> In-Reply-To: <201311020011.rA20BchL020170@svn.freebsd.org> References: <201311020011.rA20BchL020170@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 2 Nov 2013, Adrian Chadd wrote: > Log: > Teach the netgraph code to use a const char * pointer too. This actually fixes a much larger bug that was apparently accidentally exposed by new const poisoning and new smaller type bugs. > Pointy hat to: adrian Outpointed by the previous bug. > Modified: head/sys/netgraph/ng_iface.c > ============================================================================== > --- head/sys/netgraph/ng_iface.c Fri Nov 1 23:30:54 2013 (r257534) > +++ head/sys/netgraph/ng_iface.c Sat Nov 2 00:11:38 2013 (r257535) > @@ -776,7 +776,7 @@ ng_iface_rcvdata(hook_p hook, item_p ite > return (EAFNOSUPPORT); > } > if (harvest.point_to_point) > - random_harvest(&(m->m_data), 12, 2, RANDOM_NET_NG); '&(m->m_data)' is not just a pair of style bugs. It gives address of the pointer (somewhere in the mbuf header), not the address of pointed- to data, so the randomness was almost null. The style bugs are excessive parentheses and chumminess with the implementation (non-use of the accessor function mtod()). > + random_harvest(mtod(m, const void *), 12, 2, RANDOM_NET_NG); Presumably you really do want to harvest the pointed-to data and there are at least 12 bytes of it, so the semantic fix isn't backwards or a buffer overrun. > M_SETFIB(m, ifp->if_fib); > netisr_dispatch(isr, m); > return (0); const poisoning sometimes exposes garbage pointers accidentally. It seems to take a new design error (random_harvest() taking a const char * instead of a const void *) for this bug to be detected. random_harvest() used to take a void * arg and the garbage pointer was automatically converted to that. Conversion to const void * would be just as automatic. The log message says const char * but the code says const void *. The latter is correct, at least mtod() is used, since the mtod() API unfortunately requires naming a type and const void * is the best type to use here. Then futher conversions apparently occur: if there is actually a const char * anywhere, it is wrong, and if we convert this const void * to it then we must convert to const u_char * to actually access the data It is best for intermediate conversions to only go through const void * and not expand the old design error of making them go through caddr_t (for m_data). Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131102151309.A1102>