Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flow/manager: fix multi instance row tracking #12205

Closed
wants to merge 1 commit into from

Conversation

victorjulien
Copy link
Member

In multi instance flow manager setups, each flow manager gets a slice of the hash table to manage. Due to a logic error in the chunked scanning of the hash slice, instances beyond the first would always rescan the same subslice of their slice.

The pos variable that is used to keep the state of what the starting position for the next scan was supposed to be was treated as if it held a relative value. Relative to the bounds of the slice. It was however, holding an absolute position. This meant that when doing it's bounds check it was always considered out of bounds.

This patch addresses the issue by correctly handling the fact that the value is absolute.

Bug: #7365.

https://redmine.openinfosecfoundation.org/issues/7365

In multi instance flow manager setups, each flow manager gets a slice
of the hash table to manage. Due to a logic error in the chunked
scanning of the hash slice, instances beyond the first would always
rescan the same subslice of their slice.

The `pos` variable that is used to keep the state of what the starting
position for the next scan was supposed to be was treated as if it held
a relative value. Relative to the bounds of the slice. It was however,
holding an absolute position. This meant that when doing it's bounds
check it was always considered out of bounds.

This patch addresses the issue by correctly handling the fact that the
value is absolute.

Bug: OISF#7365.
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.17%. Comparing base (e9173f3) to head (8dc0052).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12205      +/-   ##
==========================================
- Coverage   83.17%   83.17%   -0.01%     
==========================================
  Files         912      912              
  Lines      257111   257111              
==========================================
- Hits       213856   213851       -5     
- Misses      43255    43260       +5     
Flag Coverage Δ
fuzzcorpus 61.01% <0.00%> (ø)
livemode 19.41% <100.00%> (-0.01%) ⬇️
pcap 44.41% <100.00%> (+0.02%) ⬆️
suricata-verify 62.77% <100.00%> (-0.01%) ⬇️
unittests 59.17% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@victorjulien
Copy link
Member Author

Replaced by #12208

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant