This is the old code in net/ipv6/ip6_output.c:
mtu -= hlen + sizeof(struct frag_hdr);
and this your suggested replacement:
if (overflow_usub(mtu, hlen + sizeof(struct frag_hdr), &mtu) || mtu <= 7) goto fail_toobig;
The problem as I see it is that the overflow_usub()
function is non-standard and requires special compiler support to generate reasonably efficient code. That function hasn't been used anywhere else in the code base before. Also, the code uses two conditionals. Consider this replacement:
if (mtu < hlen + sizeof(struct frag_hdr) + 8) goto fail_toobig; mtu -= hlen + sizeof(struct frag_hdr);
It takes the same number of lines, doesn't need a little-known helper function, is clearer in its intent and, I think, is easier to read. There could still be overflow issues if the "hlen + xyz" expression overflows, but the "overflow_usub()" code has that same issue.
Also, we are near to rc7 time, and we don't want conflicts at this point.
Thank you for submitting the code, but, for all the aforementioned reasons, I have to reject it and will do so for similar code in the future.
Disclaimer: I am not a kernel programmer, so I may have am certain I have misinterpreted some of the technical aspects of Linus’s original
post. (update at 8:44 p.m.)