Skip to content

logging: prevent double-free multi-threaded logging v1.1 - #16137

Closed
lukashino wants to merge 1 commit into
OISF:mainfrom
lukashino:bug/8861-double-free-output-ctx-v1.1
Closed

logging: prevent double-free multi-threaded logging v1.1#16137
lukashino wants to merge 1 commit into
OISF:mainfrom
lukashino:bug/8861-double-free-output-ctx-v1.1

Conversation

@lukashino

Copy link
Copy Markdown
Contributor

Follow-up of the pre-approved #16057 (got blocked by SV tests)

When multithreaded output was enabled, all threads freed shallow-copy of sensor and prefix names.

This commit adds a guard so that only the parent frees the allocated buffers.

Ticket: https://redmine.openinfosecfoundation.org/issues/8861

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

Describe changes:
v1.1:

  • rebased

When multithreaded output was enabled, all threads freed
shallow-copy of sensor and prefix names.

This commit adds a guard so that only the parent frees the
allocated buffers.

Ticket: 8861

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a shutdown/cleanup double-free in multithreaded logging by ensuring that only the “parent” LogFileCtx frees shared (shallow-copied) buffers like prefix and sensor_name, preventing leaf thread contexts from freeing the same allocations.

Changes:

  • Guard freeing of lf_ctx->prefix / lf_ctx->sensor_name so it only happens when lf_ctx->parent == NULL.
  • Keep existing threaded-leaf cleanup behavior intact while avoiding shared-buffer double free.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.09%. Comparing base (389700e) to head (19ce250).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16137      +/-   ##
==========================================
+ Coverage   83.05%   83.09%   +0.03%     
==========================================
  Files        1004     1004              
  Lines      277407   277408       +1     
==========================================
+ Hits       230410   230519     +109     
+ Misses      46997    46889     -108     
Flag Coverage Δ
fuzzcorpus 61.50% <0.00%> (-0.01%) ⬇️
livemode 18.48% <50.00%> (+0.06%) ⬆️
netns 22.84% <50.00%> (-0.06%) ⬇️
pcap 45.35% <50.00%> (-0.04%) ⬇️
suricata-verify 67.28% <100.00%> (+0.03%) ⬆️
unittests 58.53% <0.00%> (-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.

@victorjulien victorjulien added this to the 9.0 milestone Aug 28, 2026
@suricata-qa

Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 33426

@victorjulien

Copy link
Copy Markdown
Member

Merged in #16141, thanks!

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.

4 participants