Skip to content

Fix memory leak in daemon_unpackapplyfilter() error paths#1640

Open
nidu-ninja wants to merge 1 commit intothe-tcpdump-group:masterfrom
nidu-ninja:rpcapd-error-path-leak
Open

Fix memory leak in daemon_unpackapplyfilter() error paths#1640
nidu-ninja wants to merge 1 commit intothe-tcpdump-group:masterfrom
nidu-ninja:rpcapd-error-path-leak

Conversation

@nidu-ninja
Copy link
Copy Markdown

Fix a memory leak in daemon_unpackapplyfilter() where dynamically
allocated BPF instruction memory was not freed on early-return
error paths.

This change introduces a structured cleanup block to ensure
allocated memory is released when instruction reception,
validation, or filter application fails.

@infrastation
Copy link
Copy Markdown
Member

Please put the commit message into the commit rather than request description and keep the existing indentation style.

@nidu-ninja nidu-ninja force-pushed the rpcapd-error-path-leak branch from 7b563c7 to b76ffe6 Compare March 6, 2026 21:06
@infrastation
Copy link
Copy Markdown
Member

For reference, this problem has been known as Coverity CID 1641537.

static int
daemon_unpackapplyfilter(PCAP_SOCKET sockctrl, SSL *ctrl_ssl, struct session *session, uint32_t *plenp, char *errmsgbuf)
{
int status;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As @infrastation said, please don't change the indentation style - it makes it difficult to see what the real changes are.

@fxlb
Copy link
Copy Markdown
Member

fxlb commented Mar 27, 2026

Avoid merge, use rebase on top of master.

allocated BPF instruction memory was not freed on early-return
error paths.

This change introduces a structured cleanup block to ensure
allocated memory is released when instruction reception,
validation, or filter application fails.
@nidu-ninja nidu-ninja force-pushed the rpcapd-error-path-leak branch from 7dc5fea to d78dff8 Compare March 27, 2026 17:55
(EOA) as of December 31, 2017
Make pcap_compile() error messages more uniform and consistent.
Deprecate pcap_compile_nopcap().
Deprecate bpf_filter().
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a change for 1.10.7, not 1.11.0.

Fix DECnet packet filtering on big-endian hosts.
Fix various failures to reject invalid DECnet primitives.
Require "vpi" and "vci" values to be within valid ranges.
Initialize the scratch memory store to 0.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the only change that you should be making to the CHANGES file.

In "gateway" negate the host(s), but not the protocol.
Reject "gateway" within MPLS, VXLAN or Geneve.
In "net <n> mask <m>" catch ENOMEM for the "m" too.
Match both byte orders for the AF_ value when filtering DLT_NULL
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should not be removed.

Discuss Linux BPF extensions in the man pages.
Note endianness in pcap_compile(3PCAP) and pcap_lookupnet(3PCAP).
man: Document devices, interfaces and "any" better.
Remove list of OSes that support "ipv6-icmp"; all the ones we
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is another 1.10.7 change that should not be added here.

Remove list of OSes that support "ipv6-icmp"; all the ones we
support appear to do so.
Add a README.qnx.md file.
Improve dcumentation of TLS supprt in rpcapd man page.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't remove existing CHANGES entries.

QNX:
Disable zero-copy BPF to work around portability issues.
Use "unix.h" instead of the missing <sysexits.h>.
RDMA:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't remove existing CHANGES entries and don't remove the 1.10.7 set of changes.

@guyharris
Copy link
Copy Markdown
Member

It appears that you don't have the current version of the CHANGES file as the baseline for your change. Please fix that.

@fxlb
Copy link
Copy Markdown
Member

fxlb commented Mar 27, 2026

Whitespaces change problem. Please fix.

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