Skip to content

next/1470/80x/20260831/v1 - #16146

Merged
victorjulien merged 5 commits into
OISF:main-8.0.xfrom
victorjulien:next/1470/80x/20260831/v1
Aug 31, 2026
Merged

next/1470/80x/20260831/v1#16146
victorjulien merged 5 commits into
OISF:main-8.0.xfrom
victorjulien:next/1470/80x/20260831/v1

Conversation

@victorjulien

Copy link
Copy Markdown
Member

inashivb and others added 5 commits August 31, 2026 07:01
The fn to flush the ring is supposed to flush the entire ring buffer but
it only resets the buffer up to the "length" bytes. Fix this calculation
to correctly reflect the number of bytes that need to be reset.

Ticket: 8826
(cherry picked from commit 389700e)
Setting a cpu-affinity path with --set, like

  --set threading.cpu-affinity.worker-cpu-set.threads=28

creates an intermediate node under cpu-affinity whose val is NULL.

With a legacy list format cpu-affinity, AffinitySetupLoadFromConfig()
red the set name from the node val and passed it to
GetAffinitySetName(), which then dereferenced the NULL pointer.

Address it by simply checking for the NULL.

Ticket: OISF#6735.
(cherry picked from commit 54504ed)
Zero the result when a byte_math right shift count reaches 64, the
width of the uint64_t being shifted, so the operation no longer
depends on behavior C11 6.5.7p3 leaves undefined. The left shift case
has done this since 473ca6d; the right shift case was left
unguarded.

DetectByteMathDoMatch() shifted by whatever count it was handed. On
x86_64 the hardware masks the count to its low six bits, so a count of
64 became a shift of 0 and returned the extracted value unchanged
instead of 0. That value is stored in det_ctx->byte_values[] and feeds
any byte_test, isdataat, or content offset later in the signature, so
the signature's verdict follows from an arithmetic result the standard
does not define.

The count reaches the shift from the wire. When byte_math names a
variable for rvalue, DetectEngineContentInspectionInternal() reads it
out of det_ctx->byte_values[] at
detect-engine-content-inspection.c:614, where a preceding byte_extract
stored bytes taken from the payload, so one payload byte of 0x40 sets
the count to 64.

Issue: 8845
Issue: 8902
(cherry picked from commit e5d035f)
Warn from DetectByteMathSetup() when byte_math pairs << or >> with a
literal rvalue of 64 or more, naming the signature by sid. Such a
shift gives 0 for every packet: DetectByteMathDoMatch() zeroes the
result once the count reaches 64, the width of the uint64_t being
shifted. rvalue was bounded only to the u32 range, so the rule loaded
and ran a shift whose result was 0 whatever the packet held.

The rule still loads. main rejects it in 89d09c4, the commit this
one is derived from, but a released branch should not stop loading
rules that loaded before. Passing byte_math to --strict-rule-keywords
turns the warning into a load failure, the same escalation classtype
and reference apply to values they cannot check.

A variable rvalue draws no warning. It resolves to a preceding
byte_extract or byte_math result, whose value is known only once the
rule runs, so the guard in DetectByteMathDoMatch() stays the only
check on that path.

Issue: 8845
Issue: 8902
(cherry picked from commit 89d09c4)
Record beside the existing note about division by zero that << and >>
give 0 when rvalue is 64 or more, that a rule giving such an rvalue as
a number still loads with a warning, and that --strict-rule-keywords
makes it fail to load instead. rvalue can name a byte_extract or
byte_math variable rather than a number, and then the count is not
known until the rule runs, so the rule text does not say whether it
will reach 64.

The note on main stops at the result, because 89d09c4 rejects such
a rule there instead of warning about it.

Issue: 8845
Issue: 8902
(cherry picked from commit 4a64778)
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.42105% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.74%. Comparing base (3b04b76) to head (d22b91c).
⚠️ Report is 5 commits behind head on main-8.0.x.

Additional details and impacted files
@@             Coverage Diff             @@
##           main-8.0.x   #16146   +/-   ##
===========================================
  Coverage       81.73%   81.74%           
===========================================
  Files            1017     1017           
  Lines          277278   277367   +89     
===========================================
+ Hits           226628   226724   +96     
+ Misses          50650    50643    -7     
Flag Coverage Δ
fuzzcorpus 63.88% <32.00%> (+0.01%) ⬆️
livemode 18.64% <4.00%> (-0.01%) ⬇️
netns 20.16% <0.00%> (-0.01%) ⬇️
pcap 44.49% <0.00%> (-0.02%) ⬇️
suricata-verify 65.30% <52.00%> (+0.05%) ⬆️
unittests 58.57% <81.05%> (+<0.01%) ⬆️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suricata-qa

Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 33469

@jufajardini jufajardini left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent with original, approved PRs, CI checks pass.

@victorjulien
victorjulien merged commit d22b91c into OISF:main-8.0.x Aug 31, 2026
61 checks passed
@victorjulien
victorjulien deleted the next/1470/80x/20260831/v1 branch August 31, 2026 13:06
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.

5 participants