Re: CVS commit: src/external/bsd/dhcpcd/dist/src

[Martin Husemann <martin%duskware.de@localhost>]

On Fri, Aug 03, 2018 at 02:47:53PM +0200, Kamil Rytarowski wrote:
> > Further if there ever was a potential problem from this line ...
> > 
> > 	*len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
> > then
> > 	*len = (size_t)ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);

> It was a build time error generated by GCC. The compiler detected that
> both sizeof() could be large enough to overflow the result returned from
>  ntohs(3). And overflowing a signed integer is Undefined Behavior.

But we do not do this here.

> This change points to the compiler that the code is safe.

What exactly makes the code safe now? If ntohs(p->ip.ip_len) <
(sizeof(p->ip) + sizeof(p->udp)) then we are now in even more serious
trouble.

Does splitting the term help?

	uint16_t hdr_size = sizeof(p->ip) - sizeof(p->udp);
	uint16_t pkt_size = ntohs(p->ip.ip_len);
	KASSERT(pkt_size > hdr_size);
	*len = pkt_size > hdr_size ? pkt_size-hdr_size : 0;

or something like that?

Martin