Skip to content

Make sure open() and read() succeeded - #10273

Merged
bryancall merged 1 commit into
apache:masterfrom
duke8253:master-check_fd
Oct 9, 2023
Merged

bryancall merged 1 commit into
apache:masterfrom
duke8253:master-check_fd

Conversation

@duke8253

@duke8253 duke8253 commented Aug 23, 2023

Copy link
Copy Markdown
Contributor

@duke8253 duke8253 added this to the 10.0.0 milestone Aug 23, 2023
@duke8253 duke8253 self-assigned this Aug 23, 2023
@duke8253 duke8253 changed the title make sure open() succeeded Make sure open() succeeded Aug 23, 2023
@duke8253 duke8253 changed the title Make sure open() succeeded Make sure open() and read() succeeded Aug 23, 2023
@duke8253
duke8253 force-pushed the master-check_fd branch 2 times, most recently from 8bb5fd2 to b718b6a Compare August 24, 2023 15:30
@duke8253
duke8253 requested a review from shinrich August 24, 2023 15:31
@duke8253

Copy link
Copy Markdown
Contributor Author

[approve ci]

masaori335
masaori335 previously approved these changes Aug 28, 2023

@masaori335 masaori335 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, please fix the sign-compare warning.

experimental/ssl_session_reuse/src/ssl_init.cc:118:60: error: comparison of integer expressions of different signedness: 'long int' and 'std::__cxx11::basic_string<char>::size_type' {aka 'long unsigned int'} [-Werror=sign-compare]
  118 |       if (read_len > 0 && read_len <= buffSize && read_len <= key_data.length()) {
      |                                                   ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~

@bryancall
bryancall merged commit 26affda into apache:master Oct 9, 2023
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

Projects

None yet

4 participants