Skip to content

Commit c7dd282

Browse files
committed
defrag: drop unsigned underflow in RB_NFIND search key
The seek into the fragment tree used `frag_offset - 1` as the key for RB_NFIND. For the first fragment of an IP datagram `frag_offset` is 0 and the uint16_t subtraction wraps to 65535. The wrap is harmless in practice (no fragment can have offset 65535 since IPv4 limits the field to 8191 << 3 = 65528 and IPv6 follows the same alignment), so RB_NFIND returns NULL and the existing RB_MIN fallback walks the tree forward. The end result is correct but the path is reached by accident. The `-1` does nothing useful for any other input either: fragment offsets are always multiples of 8 (IPV4_GET_RAW_FRAGOFFSET << 3 for IPv4, the fragment header offset field for IPv6), so no fragment can ever have offset == frag_offset - 1, and RB_NFIND returns the same node for keys `frag_offset` and `frag_offset - 1` on every realistic input. Drop the subtraction. RB_NFIND's inclusive semantics give the same neighbour selection for non-zero frag_offset and a defined result for frag_offset == 0. The RB_PREV/RB_MIN fallback logic that follows is left intact: in the no-strict-greater case it still starts at the tree minimum and walks forward, which is the only safe choice given the defrag policies do not enforce a non-overlapping invariant on offset+data_len (overlaps are handled through ltrim, not by trimming data_len).Bug: #8232.
1 parent 367ca7f commit c7dd282

1 file changed

Lines changed: 1 addition & 1 deletion

File tree

src/defrag.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker,
654654

655655
if (!RB_EMPTY(&tracker->fragment_tree)) {
656656
Frag key = {
657-
.offset = frag_offset - 1,
657+
.offset = frag_offset,
658658
};
659659
next = RB_NFIND(IP_FRAGMENTS, &tracker->fragment_tree, &key);
660660
if (next == NULL) {

0 commit comments

Comments
 (0)