Hello. Please forgive my delay.
16:58:32.290407 IP 10.12.48.10.33088 > 10.12.48.32.3285: UDP, length 2959 16:58:32.290410 IP 10.12.48.10 > 10.12.48.32: udp 16:58:32.290412 IP 10.12.48.10 > 10.12.48.32: udp
The third fragment here is less than 8 bytes of payload, and this triggers the bug.
I don't know yet what are the side effects of my patch.
Ok, I restudied the flow.
- if (offset8 + (len / 8) <= h - payload) {
- if (offset8 + (len / 8) < h - payload) { /* no overlap with holes (dup fragment?) */ return NULL;
offset8 is the start of this fragment, as 8-byte-items, and h is the current hole, the one that might be relevant. Since everything before "h" is already filled, a fragment that ends where this hole starts is duplicate, thus "<=".
Here len/8 is 0, so you force accepting this trailing fragment, but you'll also accept duplicate packets. Looking at how following code behaves in this case (it assumes some hole or part of is filled), the "overlaps with initial part of the hole" will trigger, and the hole will be rewritten unchanged.
So I don't see bad side effects in your fix, but if anyone will re-read the code (looking for another bug, for example) after your change it will be hard to understand (they'll suggest reverting the change for example).
I'd suggest to still discard duplicate fragments, by turning
if (offset8 + (len / 8) <= h - payload)
to /* last fragment may be 1..7 bytes, the "+7" forces acceptance */ if (offset8 + ((len + 7) / 8) <= h - payload)
or something along these lines. Will you please post a patch?
Thanks a lof for your report and fix
/alessandro