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?