Project

General

Profile

Actions

Bug #7252

closed

stream/reassemble: GetBlock implies gap without searching the entire tree for block

Added by Shivani Bhardwaj about 2 months ago. Updated about 1 month ago.

Status:
Rejected
Priority:
Normal
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

GetBlock fn has this logic:

      for ( ; blk != NULL; blk = SBB_RB_NEXT(blk)) {
         if (blk->offset >= offset) {
              return blk;         
         } else if ((blk->offset + blk->len) > offset) {
              return blk;         
          }                       
      }                           
      return NULL; 

This means that the moment a block with an offset greater than the asked offset was found, it was returned.

In the caller, therefore, the following is done:

          /* block past out offset */
          else if (blk->offset > offset) {
              SCLogDebug("gap, want data at offset %"PRIu64", " 
                      "got data at %"PRIu64". GAP of size %"PRIu64,
                      offset, blk->offset, blk->offset - offset);
              *data = NULL; 
              *data_len = blk->offset - offset;
           }

and then, the data offset is adjusted as per some gap handling logic.

This is incorrect because the point of GetBlock fn is to get the block containing a given offset. Entire tree should have been searched for the given offset instead of returning the first block greater than equal to the given offset.
Note that if a block has offset equal to the given offset, it is perfect. It is incorrect in the other case i.e. the block offset is greater than the given offset.

Actions #1

Updated by Shivani Bhardwaj about 1 month ago

  • Status changed from New to Rejected
  • Assignee changed from Community Ticket to Shivani Bhardwaj

This bug was based on an incorrect assumption: that sb->head points to the root of the tree. Turns out it points to the minimum value in the SBB tree so it really just makes up for a linear search starting at the lowest value node.

Rejecting ticket as invalid.
Tested and verified after creating a pcap with appropriate gaps matching the criteria where I thought this would be buggy.

Actions

Also available in: Atom PDF