detect/byte: cache byte values in tx state by jlucovsky · Pull Request #15175 · OISF/suricata

jlucovsky · 2026-04-07T14:07:28Z

Continuation of #15085

Add per-transaction byte value caching so that variables produced by byte_extract and byte_math persist across buffer and direction boundaries.

Without this change, bidirectional (=>) rules that produce a byte variable in a toserver buffer and consume it in a toclient buffer fail when HTTP/1.1 pipelining creates multiple transactions. All TXs share the same det_ctx->byte_values, and a later TX's toserver inspection clobbers values before an earlier TX's toclient inspection runs.

The fix saves byte values to the per-TX DetectEngineState after extraction, restores them before each content inspection pass, clears them when reseting a TX and frees the memory when DetectEngineState is destroyed, transaction is reset, or the DetectEngineThreadCtx is uninitialized.

Link to ticket: https://redmine.openinfosecfoundation.org/issues/7801

Describe changes:

detect-engine-state.c/.h: Add byte_values array to DetectEngineState with save/restore functions, cleared/freed on reset, free on destroy
detect-engine-content-inspection.c: Restore cached values at start of inspection, save after byte_extract/byte_math
detect-engine.c, detect.h: Track byte_values_len in DetectEngineThreadCtx
rust/sys/src/sys.rs: Add new fields to bindgen struct
Documentation updates in differences-from-snort.rst, payload-keywords.rst, and devguide/transactions.rst

Updates:

Moved from issue 1412 to 7801 as focus is on cross buffer-variable usage
Adjust positioning of byte_value variables to reduce holes in struct (see scan-build in detect/byte: cache byte values in tx state #15027)
Skip caching for unidirectional rules since all buffers are inspected in a single TX.
Clear on engine reload
Removed dynamic growth with a single alloc as the size is static.
Address review comments (see detect/byte: cache byte values in tx state #15084)

Provide values to any of the below to override the defaults.

To use a Suricata-Verify or Suricata-Update pull request,
link to the pull request in the respective _BRANCH variable.
Leave unused overrides blank or remove.

SV_REPO=
SV_BRANCH=OISF/suricata-verify#2968
SU_REPO=
SU_BRANCH=

- Add detection-state storage/retrieval for byte keyword values. - Restore cached values before content inspection and update cache after extraction/math. - Clear cached values on tx state reset and free them during state destruction. Issue: 7801

Update documentation to reflect that variables produced by byte_extract and byte_math can now be used across different buffers and direction boundaries. The per-transaction cache ensures produced values persist when the engine switches between buffers or evaluates bidirectional rules. - differences-from-snort: replace same-buffer limitation with cross-buffer support description - payload-keywords: add cross-buffer notes and example to byte_extract and byte_math sections - devguide/transactions: add per-transaction byte value cache section describing the save/restore mechanism Issue: 7801

codecov · 2026-04-07T14:44:27Z

Codecov Report

❌ Patch coverage is 83.33333% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.72%. Comparing base (336b50a) to head (985b304).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15175      +/-   ##
==========================================
+ Coverage   82.71%   82.72%   +0.01%     
==========================================
  Files         993      993              
  Lines      271737   271795      +58     
==========================================
+ Hits       224772   224852      +80     
+ Misses      46965    46943      -22

Flag	Coverage Δ
fuzzcorpus	`61.11% <31.66%> (-0.02%)`	⬇️
livemode	`18.35% <6.66%> (-0.01%)`	⬇️
netns	`22.61% <8.33%> (+<0.01%)`	⬆️
pcap	`45.26% <10.00%> (-0.07%)`	⬇️
suricata-verify	`66.29% <81.66%> (+0.01%)`	⬆️
unittests	`58.85% <13.33%> (-0.02%)`	⬇️

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 · 2026-04-07T16:35:04Z

WARNING:

field	baseline	test	%
	SURI_TLPR1_stats_chk
.app_layer.flow.ftp-data	697	671	96.27%

Pipeline = 30831

catenacyber · 2026-04-09T11:30:27Z

Question about tickets and scope :

does this PR solve 1412 as well ?
can it also solve https://redmine.openinfosecfoundation.org/issues/7197 ?

You said

similar problem with a different variable type.

Is it a simple fixup to add ? Or a more complete one

Is there not some logic needed to forbid rules that try to use byte values before they were set ? see detect/byte: cache byte values in tx state #15084 (comment)

You said

I'll file a ticket for this case

Did you file a ticket ? I do not see one linked to https://redmine.openinfosecfoundation.org/issues/7801

catenacyber · 2026-04-09T12:35:53Z

+     * (by DetectEngineStateFree or DetectEngineThreadCtxDeinit). */
+    uint64_t *tmp = det_ctx->byte_values;
+    det_ctx->byte_values = state->byte_values;
+    state->byte_values = tmp;


So, this is incorrect even if it is memory-safe

See OISF/suricata-verify#3021 latest commit test 7801-10

We have 2 txs that should match because the value in file.data is > than the value extracted in the uri
But we have only one match

Debug-print in bytetest shows that we use 88 for both txs !

This is because this pointers swap is not reset.

So, after one packet which calls DetectEngineContentInspectionRestoreByteValues
detctx has now the tx0 values, tx0 has now tx1 values, tx1 has now the dectx values

So, if we see another later packet which calls DetectEngineContentInspectionRestoreByteValues, we do not use the right values for the right tx

Good catch -- the swap doesn't work for mult. packets. A memcpy would, however, work and i'm looking at that.

Thanks for the test case -- I'll add case 10 to my s-v PR.

I closed OISF/suricata-verify#3021 as you took my test 10 in your own PR

catenacyber

See inline comment

jlucovsky · 2026-04-09T12:55:49Z

Question about tickets and scope :

does this PR solve 1412 as well ?

can it also solve https://redmine.openinfosecfoundation.org/issues/7197 ?

You said

similar problem with a different variable type.

Is it a simple fixup to add ? Or a more complete one

Is there not some logic needed to forbid rules that try to use byte values before they were set ? see detect/byte: cache byte values in tx state #15084 (comment)

You said

I'll file a ticket for this case

Did you file a ticket ? I do not see one linked to https://redmine.openinfosecfoundation.org/issues/7801

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

jlucovsky · 2026-04-12T13:18:28Z

Continued in #15195

jlucovsky added 2 commits April 7, 2026 09:42

jlucovsky requested review from jasonish, jufajardini and victorjulien as code owners April 7, 2026 14:07

jlucovsky mentioned this pull request Apr 7, 2026

detect/byte: cache byte values in tx state #15084

Closed

catenacyber reviewed Apr 9, 2026

View reviewed changes

catenacyber requested changes Apr 9, 2026

View reviewed changes

jlucovsky mentioned this pull request Apr 12, 2026

detect/byte: cache byte values in tx state #15195

Closed

jlucovsky closed this Apr 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

detect/byte: cache byte values in tx state#15175

detect/byte: cache byte values in tx state#15175
jlucovsky wants to merge 2 commits into
OISF:mainfrom
jlucovsky:7801/4

jlucovsky commented Apr 7, 2026

Uh oh!

codecov Bot commented Apr 7, 2026 •

edited

Loading

Uh oh!

suricata-qa commented Apr 7, 2026

Uh oh!

catenacyber commented Apr 9, 2026

Uh oh!

catenacyber Apr 9, 2026

Uh oh!

jlucovsky Apr 11, 2026

Uh oh!

jlucovsky Apr 12, 2026

Uh oh!

catenacyber Apr 20, 2026

Uh oh!

catenacyber left a comment

Uh oh!

jlucovsky commented Apr 9, 2026

Uh oh!

jlucovsky commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Conversation

jlucovsky commented Apr 7, 2026

Provide values to any of the below to override the defaults.

Uh oh!

codecov Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

suricata-qa commented Apr 7, 2026

Uh oh!

catenacyber commented Apr 9, 2026

Uh oh!

catenacyber Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

jlucovsky Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

jlucovsky Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

catenacyber Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

catenacyber left a comment

Choose a reason for hiding this comment

Uh oh!

jlucovsky commented Apr 9, 2026

Uh oh!

jlucovsky commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Apr 7, 2026 •

edited

Loading