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
