Skip to content

pcap-file: skip setvbuf on non-seekable streams (v3) - #15629

Closed
ssam18 wants to merge 1 commit into
OISF:mainfrom
ssam18:pcap-file-setvbuf-stdin-v3
Closed

pcap-file: skip setvbuf on non-seekable streams (v3)#15629
ssam18 wants to merge 1 commit into
OISF:mainfrom
ssam18:pcap-file-setvbuf-stdin-v3

Conversation

@ssam18

@ssam18 ssam18 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Make sure these boxes are checked accordingly before submitting your Pull Request -- thank you.

Contribution style:

Our Contribution agreements:

Changes (if applicable):

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

Replaces #15562.

Changes since #15562 (all addressing @jlucovsky's review)

  • Accepted pcap-file.buffer-size values are now exactly 0 (disables setvbuf buffering) or PCAP_FILE_BUFFER_SIZE_MIN (4 KiB) to PCAP_FILE_BUFFER_SIZE_MAX (64 MiB); values in between are rejected with a warning naming both valid forms.
  • PCAP_FILE_BUFFER_SIZE_MIN is restored to 4096U and referenced again by the validation.
  • The setvbuf return value is checked with != 0 since any non-zero value (not just negative) indicates an error.
  • When buffering is disabled via 0, the log now says so explicitly instead of "will use 0 buffer size".
  • User Guide updated to document the valid values as 0 or 4 KiB to 64 MiB.
  • Rebased on current main.

Description

Reading a pcap from /dev/stdin or a named pipe regressed in 8.0.0 with the setvbuf change in 7b730c2 and currently fails with failed to get first packet timestamp. pcap_next_ex(): -1. The reason is that InitPcapFile calls setvbuf on the FILE* underlying the pcap handle after libpcap has already consumed the pcap header, and on a non seekable fd glibc cannot recover from that and the very next read returns -1. This change detects non regular files via fstat on the underlying fd and skips setvbuf for that handle, so reading from stdin, named pipes, and other non seekable sources keeps working. pcap-file.buffer-size = 0 remains available as an explicit opt out, matching the workaround proposed on the ticket.

Describe changes:

  • In InitPcapFile, fstat the fd behind pcap_file(handle) and skip setvbuf when the file is not a regular file. An info log explains the skip.
  • Accept buffer-size 0 as an explicit setvbuf opt-out while keeping the 4 KiB minimum for actual buffering lengths.
  • On a buffer-size parse error, retain the default instead of setting it to 0.
  • Treat any non-zero setvbuf return value as an error.
  • Verified locally that tcpdump piping a pcap into /dev/stdin and reading from a named pipe both now complete with 2 packets read and 0 errors. Regular file reads are unchanged.

SV_BRANCH=OISF/suricata-verify#3108

Reading a pcap from /dev/stdin or a named pipe currently fails with "failed to get first packet timestamp. pcap_next_ex(): -1" because InitPcapFile calls setvbuf on the FILE* underlying the pcap handle after libpcap has already consumed the pcap header. On a non-seekable fd glibc cannot recover from that and the next read returns -1.

Detect non-regular files via fstat and skip setvbuf in that case so the read keeps working on pipes, fifos and stdin.

Accept pcap-file.buffer-size values of 0, which disables setvbuf buffering as an explicit opt-out, or PCAP_FILE_BUFFER_SIZE_MIN (4 KiB) to PCAP_FILE_BUFFER_SIZE_MAX (64 MiB). Treat any non-zero setvbuf return value as an error, not just negative values.

When pcap-file.buffer-size fails to parse, retain the default buffer size instead of falling through and setting it to 0. The branches are now mutually exclusive so only one of the parse-error, accepted, or out-of-range messages is logged.

Update the user guide: --pcap-file-buffer-size now documents valid values of 0 (disables setvbuf buffering) or 4 KiB to 64 MiB, and pcap-file.rst notes that 0 is the opt-out for non-seekable sources such as stdin and named pipes.
Bug: OISF#8464.

@jlucovsky jlucovsky 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.

Looks good

Minor but important -- please adjust the copyright dates in all source modules to include 2026.

@ssam18

ssam18 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @jlucovsky — copyright dates updated to include 2026 in the v4 PR #15631 (source-pcap-file.c was still 2007-2016; the helper already had 2026). Closing in favor of #15631.

@ssam18 ssam18 closed this Jun 12, 2026
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