Skip to content

healthchecks: reference count status file data - #13432

Merged
bneradt merged 1 commit into
apache:masterfrom
bneradt:healthchecks-use-after-free
Jul 31, 2026
Merged

healthchecks: reference count status file data#13432
bneradt merged 1 commit into
apache:masterfrom
bneradt:healthchecks-use-after-free

Conversation

@bneradt

@bneradt bneradt commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Replaced file data was retired onto a freelist and freed after a
timeout, but the deadline was computed from a timestamp taken before
the blocking inotify read. That timestamp is stale by however long the
watcher waited for an event, so the deadline was routinely already in
the past and the data was freed while transactions still referenced it.

The patch addresses this issue by holding the data in a shared_ptr and
letting each transaction pin its own snapshot. The data now lives
exactly as long as it is referenced, which removes both the freelist
and the guess about how long a transaction can last.

Fixes: #8735

Copilot AI review requested due to automatic review settings July 27, 2026 16:27
@bneradt bneradt added this to the 11.0.0 milestone Jul 27, 2026
@bneradt bneradt self-assigned this Jul 27, 2026

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread plugins/healthchecks/healthchecks.cc
Comment thread plugins/healthchecks/healthchecks.cc
moonchen
moonchen previously approved these changes Jul 27, 2026

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

Thanks for fixing this.

Copilot AI review requested due to automatic review settings July 29, 2026 18:56
@bneradt
bneradt force-pushed the healthchecks-use-after-free branch from 5429a3b to 25460a7 Compare July 29, 2026 18:56

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Replaced file data was retired onto a freelist and freed after a
timeout, but its deadline came from a timestamp taken before a blocking
inotify read. That stale deadline let transactions retain references to
data that was already freed. Files exactly 16 KiB long were also
reported as empty because a final zero-byte read overwrote the saved
length.

This holds each immutable snapshot in an atomic shared_ptr. Every
transaction pins its snapshot, so data lives exactly as long as it is
referenced, without a freelist or request-path mutex. This also
preserves the last successful file-read length and adds AuTest coverage
for concurrent replacement and the 16 KiB boundary.

Fixes: apache#8735

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bneradt
bneradt force-pushed the healthchecks-use-after-free branch from 25460a7 to 6e41874 Compare July 29, 2026 20:38
Copilot AI review requested due to automatic review settings July 29, 2026 20:38

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@bneradt
bneradt merged commit 16bd59a into apache:master Jul 31, 2026
15 checks passed
@bneradt
bneradt deleted the healthchecks-use-after-free branch July 31, 2026 22:06
@github-project-automation github-project-automation Bot moved this to For v10.1.1 in ATS v10.1.x Jul 31, 2026
@github-project-automation github-project-automation Bot moved this to For v10.2.0 in ATS v10.2.x Jul 31, 2026
cmcfarlen pushed a commit that referenced this pull request Aug 4, 2026
Replaced file data was retired onto a freelist and freed after a
timeout, but its deadline came from a timestamp taken before a blocking
inotify read. That stale deadline let transactions retain references to
data that was already freed. Files exactly 16 KiB long were also
reported as empty because a final zero-byte read overwrote the saved
length.

This holds each immutable snapshot in an atomic shared_ptr. Every
transaction pins its snapshot, so data lives exactly as long as it is
referenced, without a freelist or request-path mutex. This also
preserves the last successful file-read length and adds AuTest coverage
for concurrent replacement and the 16 KiB boundary.

Fixes: #8735
(cherry picked from commit 16bd59a)
@cmcfarlen cmcfarlen moved this from For v10.2.0 to Picked v10.2.0 in ATS v10.2.x Aug 4, 2026
@cmcfarlen cmcfarlen modified the milestones: 11.0.0, 10.2.0 Aug 4, 2026
@cmcfarlen

Copy link
Copy Markdown
Contributor

Cherry-picked to the 10.2.x branch as f1b771c for the 10.2.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: For v10.1.1
Status: Picked v10.2.0

Development

Successfully merging this pull request may close these issues.

[Healthcheck] ASAN reports use after free

4 participants