Skip to content

pcap-file: synchronize PCAP filename output v4#13656

Closed
lukashino wants to merge 3 commits into
OISF:masterfrom
lukashino:bug/5255-pcap-filename-race-v4
Closed

pcap-file: synchronize PCAP filename output v4#13656
lukashino wants to merge 3 commits into
OISF:masterfrom
lukashino:bug/5255-pcap-filename-race-v4

Conversation

@lukashino

Copy link
Copy Markdown
Contributor

Follow-up of #13653
Link to ticket: https://redmine.openinfosecfoundation.org/issues/5255

Describe changes:
v4:

  • added unix socket capture mode to the PCAP file tracking functions
  • NULL check TX list (random segfault found when playing around with UNIX socket)

v3:

  • fix SCReturn to SCReturnPtr

v2:

  • Add runmode guard to PCAP file-related functions to fix running in other capture modes since pcap_v is in union with other capture modes.

v1:

  • added file structure tracked by packets/flows to report true filename

When RX thread was reading packets from multiple PCAPs, it held
internally pcap_filename variable of the currently processed PCAP.
This variable, accessed through PcapFileGetFilename(), was used to add
pcap_filename property to all eve.json events by all threads.

However, as RX thread is connected to Worker/Detect threads with queues,
packets of previous PCAPs were still in queues, while RX thread was
reading different PCAPs. This is a synchronizaiton issue.

PcapFileInfo structure is used to reference the PCAP file name. RX
thread associates the structure with all packets, Workers in turn
associate the flow with the filename. PcapFileGetFilename() is still
used when reporting stats events.

Lukas Sismis and others added 3 commits July 28, 2025 13:50
PcapFileInfo is a structure which will be referenced by packets/flows.

Ticket: 5255
When RX thread was reading packets from multiple PCAPs, it held
internally pcap_filename variable of the currently processed PCAP.
This variable, accessed through PcapFileGetFilename(), was used to add
pcap_filename property to all eve.json events by all threads.

However, as RX thread is connected to Worker/Detect threads with queues,
packets of previous PCAPs were still in queues, while RX thread was
reading different PCAPs. This is a synchronizaiton issue.

PcapFileInfo structure is used to reference the PCAP file name. RX
thread associates the structure with all packets, Workers in turn
associate the flow with the filename. PcapFileGetFilename() is still
used when reporting stats events.

Ticket: 5255
When reading with Unix socket the same file over and over again,
Suricata crashed when it did not have permissions to write
to a logging directory.
Comment thread src/output-tx.c
static uint32_t OutputTxLoggerGetActiveCount(void)
{
uint32_t cnt = 0;
if (list == NULL) {

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.

This is done better in #13616 (ie with ticket number in message) ;-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks, I'll retract the commit.

@lukashino

Copy link
Copy Markdown
Contributor Author

New PR in #13657

@lukashino lukashino closed this Jul 28, 2025
@codecov

codecov Bot commented Jul 28, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.45205% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.71%. Comparing base (6bbba95) to head (0cf6a74).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13656      +/-   ##
==========================================
+ Coverage   83.69%   83.71%   +0.01%     
==========================================
  Files        1011     1012       +1     
  Lines      275071   275121      +50     
==========================================
+ Hits       230230   230312      +82     
+ Misses      44841    44809      -32     
Flag Coverage Δ
fuzzcorpus 62.86% <68.49%> (+0.03%) ⬆️
livemode 19.01% <23.28%> (-0.17%) ⬇️
pcap 44.70% <68.49%> (-0.04%) ⬇️
suricata-verify 65.09% <78.08%> (+<0.01%) ⬆️
unittests 59.18% <17.80%> (-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.

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.

2 participants