diff --git a/rust/src/smb/files.rs b/rust/src/smb/files.rs index 71f23a6db0..6f2f1f10db 100644 --- a/rust/src/smb/files.rs +++ b/rust/src/smb/files.rs @@ -113,6 +113,7 @@ impl SMBState { tx.id, String::from_utf8_lossy(file_name) ); + self.file_tx_count += 1; self.transactions.push_back(tx); self.transactions.back_mut() } @@ -213,18 +214,11 @@ impl SMBState { } else { self.file_tc_left }; - let file_handle = if direction == Direction::ToServer { - self.file_ts_guid.to_vec() - } else { - self.file_tc_guid.to_vec() - }; - - let ssn_gap = self.ts_ssn_gap | self.tc_ssn_gap; // Backstop: cap the OOO queue on every open file tx (an interleaved - // handle can hide a first file's queue), running even with chunk_left == 0. - if ssn_gap { - // Last-resort cap for OOO data in gapped streams: configured - // size/count (1 GiB / 1024 when limits are off). + // handle can hide a queue); runs when the call feeds data: app-layer + // OOO needs no stream gap to grow a queue, an empty call grows none. + // A queue can only exist inside a held file tx. + if !data.is_empty() && self.file_tx_count > 0 { let backstop = Self::ooo_queue_backstop(direction); let cnt_backstop = Self::ooo_queue_backstop_cnt(direction); for tx in &mut self.transactions { @@ -234,7 +228,7 @@ impl SMBState { let queued_cnt = tdf.file_tracker.get_inflight_cnt() as u64; if queued_data > backstop || queued_cnt > cnt_backstop { SCLogDebug!( - "QUEUED size {} / count {} > backstop {} / {} while we've seen GAPs. Truncating file.", + "QUEUED size {} / count {} > backstop {} / {}. Truncating file.", queued_data, queued_cnt, backstop, @@ -256,6 +250,13 @@ impl SMBState { return 0; } SCLogDebug!("chunk_left {} data {}", chunk_left, data.len()); + // the backstop above no longer needs the handle: allocate it only + // when a chunk is in flight + let file_handle = if direction == Direction::ToServer { + self.file_ts_guid.to_vec() + } else { + self.file_tc_guid.to_vec() + }; let data_to_handle_len = if chunk_left as usize >= data.len() { data.len() @@ -363,6 +364,7 @@ mod tests { #[test] fn test_ooo_queue_backstop_thresholds() { + let _lock = super::SMB_CFG_TEST_LOCK.lock().unwrap(); // defaults: 64 MiB queue caps in both directions assert_eq!(SMBState::ooo_queue_backstop(Direction::ToClient), 67108864); assert_eq!(SMBState::ooo_queue_backstop(Direction::ToServer), 67108864); @@ -399,6 +401,7 @@ mod tests { #[test] fn test_ooo_queue_backstop_cnt_thresholds() { // defaults: 64 chunks in both directions + let _lock = super::SMB_CFG_TEST_LOCK.lock().unwrap(); assert_eq!(SMBState::ooo_queue_backstop_cnt(Direction::ToClient), 64); assert_eq!(SMBState::ooo_queue_backstop_cnt(Direction::ToServer), 64); diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs index 5bd9b58abf..e9bba61952 100644 --- a/rust/src/smb/smb.rs +++ b/rust/src/smb/smb.rs @@ -743,6 +743,10 @@ pub struct SMBState { /// transactions list pub transactions: VecDeque, + /// file txs held in the transactions list: the OOO backstop scan is a + /// no-op without one, so it is skipped when this is 0 + pub(crate) file_tx_count: u32, + /// tx counter for assigning incrementing id's to tx's tx_id: u64, @@ -814,6 +818,7 @@ impl SMBState { check_post_gap_file_txs: false, post_gap_files_checked: false, transactions: VecDeque::new(), + file_tx_count: 0, tx_id: 0, dialect: 0, dialect_vec: None, @@ -863,6 +868,12 @@ impl SMBState { } } if found { + if matches!( + self.transactions[index].type_data, + Some(SMBTransactionTypeData::FILE(_)) + ) { + self.file_tx_count = self.file_tx_count.saturating_sub(1); + } SCLogDebug!( "freeing TX with ID {} TX.ID {} at index {} left: {} max id: {}", tx_id,