Cleanups/v5#15725
Closed
victorjulien wants to merge 12 commits into
Closed
Conversation
Closed
jasonish
reviewed
Jun 24, 2026
|
|
||
| SCReturnCT((pstate == NULL) ? 0 : pstate->min_id, "uint64_t"); | ||
| DEBUG_VALIDATE_BUG_ON(pstate == NULL); | ||
| SCReturnCT(pstate->min_id, "uint64_t"); |
Member
There was a problem hiding this comment.
We can still get here with a NULL...
[Switching to Thread 0x7bffcf9ff6c0 (LWP 1703692)]
0x000055555609e6f8 in AppLayerParserGetMinId (pstate=0x0) at app-layer-parser.c:730
730 SCReturnCT(pstate->min_id, "uint64_t");
(gdb) bt
#0 0x000055555609e6f8 in AppLayerParserGetMinId (pstate=0x0) at app-layer-parser.c:730
#1 0x0000555556906a4f in HttpXFFGetIP (f=0x7d1ff55ad6c0, xff_cfg=0x7c1ff527b910, dstbuf=0x7bffce9f08d0 "", dstbuflen=46)
at app-layer-htp-xff.c:186
#2 0x0000555556a8bda4 in AlertJson (tv=0x7d1ff58209c0, aft=0x7c2ff547b510, p=0x7dcff6edf280) at output-json-alert.c:724
#3 0x0000555556a8d327 in JsonAlertLogger (tv=0x7d1ff58209c0, thread_data=0x7c2ff547b510, p=0x7dcff6edf280) at output-json-alert.c:915
#4 0x0000555556ab0188 in OutputPacketLog (tv=0x7d1ff58209c0, p=0x7dcff6edf280, thread_data=0x7c1ff52aaa10) at output-packet.c:104
#5 0x00005555566adb85 in OutputLoggerLog (tv=0x7d1ff58209c0, p=0x7dcff6edf280, thread_data=0x7c1ff52aaa90) at output.c:790
#6 0x0000555556667f23 in FlowWorkerFlowTimeout (tv=0x7d1ff58209c0, p=0x7dcff6edf280, fw=0x7cfff5260040, det_ctx=0x7d6ff5370200)
at flow-worker.c:462
#7 0x000055555666586e in FlowFinish (tv=0x7d1ff58209c0, f=0x7d1ff55ad6c0, fw=0x7cfff5260040, detect_thread=0x7d6ff5370200)
at flow-worker.c:129
#8 0x0000555556665d8d in CheckWorkQueue (tv=0x7d1ff58209c0, fw=0x7cfff5260040, counters=0x7bffce76d320, fq=0x7cfff52600a8, max_work=0)
at flow-worker.c:170
#9 0x00005555566686da in FlowWorkerProcessLocalFlows (tv=0x7d1ff58209c0, fw=0x7cfff5260040, p=0x7dcff8d49680) at flow-worker.c:513
#10 0x000055555666aa3c in FlowWorker (tv=0x7d1ff58209c0, p=0x7dcff8d49680, data=0x7cfff5260040) at flow-worker.c:731
#11 0x0000555555f3aa8f in TmThreadsSlotVarRun (tv=0x7d1ff58209c0, p=0x7dcff8d49680, slot=0x7c5ff528ea40) at tm-threads.c:139
#12 0x0000555555f3a153 in TmThreadsSlotProcessPkt (tv=0x7d1ff58209c0, s=0x7c5ff528ea40, p=0x7dcff8d49680)
at /home/jason/oisf/dev/suricata/review/src/tm-threads.h:201
#13 0x0000555555f3aefa in TmThreadTimeoutLoop (tv=0x7d1ff58209c0, s=0x7c5ff528ea40) at tm-threads.c:190
#14 0x0000555555f3bc10 in SCTmThreadsSlotPacketLoopFinish (tv=0x7d1ff58209c0) at tm-threads.c:285
#15 0x0000555555f3d556 in TmThreadsSlotVar (td=0x7d1ff58209c0) at tm-threads.c:526
#16 0x00007ffff78618d9 in asan_thread_start (arg=0x7bffeb4e4000) at /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:246
#17 0x00007ffff66981b9 in start_thread (arg=<optimized out>) at pthread_create.c:454
#18 0x00007ffff671d21c in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
To repro:
- Set
app-layer.httptodetection-only - Enable
xff
Then run with the attached pcap and rule file -- which GitHub won't let me attach...
Member
Author
There was a problem hiding this comment.
Will check it, but it looks like the behavior was wrong before. Wonder if we're freeing the pstate too early in some cases.
Rename progress vars.
Don't allow trailing :
It used the alstate name where it meant tx.
Fixes: 833a738 ("http: fail tx creation if we cannot allocate user data")
Since pstate can't be NULL, remove the conditional logic.
In almost every case, if there is a alstate there is also a pstate. So remove the conditional pstate handling, and replace it by unconditionally using the pointer. Add debug validation to make sure the assumption is and stays correct. Explicitly handle the one exception in AppLayerParserParse, which follows an error before the pstate is allocated, or when pstate allocation itself fails.
victorjulien
force-pushed
the
cleanups/v5
branch
from
408e4d8 to
3d231c8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #15725 +/- ##
==========================================
- Coverage 82.96% 82.96% -0.01%
==========================================
Files 1003 1003
Lines 275031 275025 -6
==========================================
- Hits 228192 228166 -26
- Misses 46839 46859 +20
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
jasonish
approved these changes
Jun 25, 2026
|
WARNING:
Pipeline = 32254 |
|
WARNING:
Pipeline = 32262 |
Open
Member
Author
|
Clean PR in #15734. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #15640: