Skip to content

Coverity 1545947: Resource leak in header_rewrite plugin - #11386

Merged
bryancall merged 1 commit into
apache:masterfrom
bryancall:cid_1545947
May 22, 2024
Merged

bryancall merged 1 commit into
apache:masterfrom
bryancall:cid_1545947

Conversation

@bryancall

Copy link
Copy Markdown
Contributor

No description provided.

@bryancall bryancall added this to the 10.1.0 milestone May 22, 2024
@bryancall bryancall self-assigned this May 22, 2024
if (p.is_cond() && add_rule(rule)) {
rule = nullptr;
if (p.is_cond() && add_rule(rule.get())) {
rule.release();

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.

It's unfortunate that we still have to call release(). I wondered if we can std::move the rule and call release() in add_rule but it looks like the following code relies on whether rule becomes nullptr.

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.

I wanted to check to return value of add_rule() before I released the pointer. add_rule() checks to see if the rule is not a nullptr and has an operator before assigning the pointer and returning true..

@bryancall

Copy link
Copy Markdown
Contributor Author

[approve ci centos]

@bryancall
bryancall merged commit 0887836 into apache:master May 22, 2024
@cmcfarlen cmcfarlen modified the milestones: 10.1.0, 10.0.0 May 24, 2024
@cmcfarlen

Copy link
Copy Markdown
Contributor

Cherry-picked to v10.0.x

cmcfarlen pushed a commit that referenced this pull request May 24, 2024
ezelkow1 pushed a commit that referenced this pull request Sep 15, 2026
* Backport Clang 21 analyzer fixes to 9.2.x

Newer Clang releases expose ownership and error-handling issues in
9.2.x as well as file I/O performed while holding the body factory lock.

This patch adapts the applicable fixes from #13593 to the older source
layout and APIs. CMake and changes to code absent from 9.2.x are omitted.

(cherry picked from commit c31517c)

* Backport earlier Clang analyzer fixes to 9.2.x

The older branch still contains uninitialized error values and cache
volume indices flagged by the analyzer.

This patch adapts #12226 to initialize those values, verify the
freelist element size before division, and remove dead initializers.

(cherry picked from commit 0cf0f3e)

* Backport session reuse key-file checks

Missing or unreadable Redis authentication key files can leave invalid
file descriptors and read lengths in the session reuse plugin.

This patch backports the key-file checks from #10273 and closes the
file only when it was opened successfully. Configuration-file handling
is addressed separately for the older branch.

(cherry picked from commit 26affda)

* Backport header rewrite rule ownership fix

Incomplete header rewrite rules can leak when configuration parsing
ends without handing the rule to the configuration.

This patch adapts #11386 to keep temporary rules under unique ownership
until the configuration accepts them.

(cherry picked from commit 0887836)

* 9.2.x: latest clang-analyzer fixes

The Ubuntu 26.04 clang-analyzer job exposes unchecked I/O, a rule leak,
and configuration reads under a mutex in the older 9.2.x code.

This branch backports the applicable fixes from #13593, #12226, #10273,
and #11386. The remaining changes handle file and socket failures,
keep session reuse configuration I/O outside the reader lock, and make
the nonblocking eventfd operation explicit to the analyzer.

* Address review findings and isolate TLS autest

A full-size Redis key leaves no room for its terminator, and failed
configuration reloads can repeat file I/O on every lookup. Modern
Fedora policy also prevents the TLS autest from exercising TLS 1.0.

This patch tightens the key bound, restores timestamp-gated reloads,
clarifies configuration publication, and guards device-dependent
checks. The TLS test uses isolated OpenSSL settings and RSA key
exchange so all four protocol checks remain active.

---------

Co-authored-by: Hiroaki Nakamura <hnakamur@gmail.com>
Co-authored-by: Fei Deng <feid@yahooinc.com>
Co-authored-by: Bryan Call <bcall@apache.org>
Co-authored-by: bneradt <bneradt@yahooinc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: picked-10.0.0

Development

Successfully merging this pull request may close these issues.

3 participants