Date: Fri, 3 Aug 2018 02:17:33 +0000 From: "Kamil Rytarowski" <ka...@netbsd.org> Message-ID: <20180803021733.b2002f...@cvs.netbsd.org>
| GCC with -fsanitize=undefiend detects a potential overflow in the code. | Cast the return value of ntohs(3) to size_t. I don't understand the point of this change, and I believe it to be wrong and misguided. 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); will not fix it. The possible problem being if ip_len < 28 (the sum of the two sizeof's). While I didn't check this code, that possibility is usually verified elsewhere, and even if not, adding the cast changes nothing, the result is still bogus (that is, at best, the change could be masking a real bug, though that's unlikely.) This looks to be (somehow) determining that the result of ntohs(ip_len) might somehow be negative (or something, I'm not really sure what is being believed is happening) but the ntohs() result is (in all cases here) a uint16_t (either because the result is literally the arg, which is that type, or because it is the result of bswap16() which is declared that way.) The sizeof() ops make the expression at least a size_t - so unless size_t and uint16_t are the same (which might happen on a pdp-11 or similar, though even there it is unlikely I suspect) the narrower one needs to be promoted - whether the ntohs() result is implicitly promoted to size_t, or explicitly cast cannot make any difference, can it? So what exactly is being fixed here? Which behaviour is supposedly undefined? kre