Skip to content

branch-4.1: [fix](hdfs) Remove query profile from HDFS file reader - #67293

Merged
yiguolei merged 1 commit into
apache:branch-4.1from
bobhan1:fix/doris-28268-hdfs-prefetch-profile-uaf
Aug 31, 2026
Merged

branch-4.1: [fix](hdfs) Remove query profile from HDFS file reader#67293
yiguolei merged 1 commit into
apache:branch-4.1from
bobhan1:fix/doris-28268-hdfs-prefetch-profile-uaf

Conversation

@bobhan1

@bobhan1 bobhan1 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

pick #67335

Issue Number: DORIS-28268

Related PR: None

Problem Summary:

CachedRemoteFileReader::prefetch_range() submits fire-and-forget dry-run tasks. A running task keeps the cached reader and its underlying HdfsFileReader alive, but it does not keep the query RuntimeProfile alive. Because HdfsFileReader stored raw pointers to query-profile timers and counters, a delayed prefetch read could update them after the query profile had been destroyed and trigger a use-after-free.

Remove the query RuntimeProfile dependency and HDFS-specific query-profile timers/counters from HdfsFileReader. This also removes the now-unused profile passthrough from HdfsFileSystem and updates all affected call sites. Generic file-reader/file-cache statistics and process-wide HDFS bvars remain unchanged; HDFS reads and cache behavior are unchanged.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
      • clang-format --dry-run --Werror on all changed C++ files
      • git diff --check
      • ./build.sh --be -j100: the changed HDFS reader/filesystem and affected call-site translation units compiled successfully. The full local BE build is blocked by an unrelated installed Lance C header mismatch (LanceScanStatistics and statistics callback definitions are missing from the local lance.h).
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes. Query profiles no longer expose the HDFS-specific HdfsIO timer and HDFS read-statistics counters. HDFS read and file-cache behavior are unchanged.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@bobhan1
bobhan1 marked this pull request as ready for review August 28, 2026 10:35
@bobhan1
bobhan1 requested a review from yiguolei as a code owner August 28, 2026 10:35
@bobhan1

bobhan1 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

gavinchou
gavinchou previously approved these changes Aug 28, 2026
@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 0.00% (0/2) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 59.44% (25664/43178)
Line Coverage 43.92% (262338/597307)
Region Coverage 39.87% (207942/521536)
Branch Coverage 41.32% (95810/231879)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 0.00% (0/2) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 74.13% (31187/42071)
Line Coverage 58.32% (345707/592815)
Region Coverage 55.13% (288214/522815)
Branch Coverage 55.95% (129591/231613)

@gavinchou
gavinchou self-requested a review August 30, 2026 12:51
Comment thread be/src/io/fs/hdfs_file_reader.cpp Outdated
@bobhan1
bobhan1 force-pushed the fix/doris-28268-hdfs-prefetch-profile-uaf branch from 2e5a7e5 to 4e45489 Compare August 31, 2026 02:40
@bobhan1 bobhan1 changed the title [fix](file cache) Avoid accessing expired HDFS profile during prefetch [fix](hdfs) Remove query profile from HDFS file reader Aug 31, 2026
@yiguolei

Copy link
Copy Markdown
Contributor

run buildall

@bobhan1 bobhan1 changed the title [fix](hdfs) Remove query profile from HDFS file reader branch-4.1: [fix](hdfs) Remove query profile from HDFS file reader Aug 31, 2026
bobhan1 added a commit to bobhan1/doris that referenced this pull request Aug 31, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: apache#67293

Problem Summary: `CachedRemoteFileReader::prefetch_range()` submits fire-and-forget dry-run tasks that can outlive the query `RuntimeProfile`. `HdfsFileReader` stored raw pointers to query-profile timers and counters, so a delayed prefetch read could update them after the query profile had been destroyed and trigger a use-after-free. Remove the query `RuntimeProfile` dependency and HDFS-specific query-profile timers and counters from `HdfsFileReader`, remove the unused profile passthrough from `HdfsFileSystem`, and update all affected call sites. Generic file-reader and file-cache statistics plus process-wide HDFS bvars remain unchanged.

### Release note

None

### Check List (For Author)

- Test: Manual test
    - `build-support/clang-format.sh`
    - `build-support/check-format.sh`
    - `build-support/check-build-hygiene.sh`
    - `git diff --check upstream/master...HEAD`
- Behavior changed: Yes. Query profiles no longer expose the HDFS-specific `HdfsIO` timer and HDFS read-statistics counters; HDFS reads and file-cache behavior are unchanged.
- Does this need documentation: No

(cherry picked from commit 4e45489)
@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 31.58% (6/19) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 74.23% (31344/42223)
Line Coverage 58.42% (348325/596204)
Region Coverage 55.09% (289938/526346)
Branch Coverage 55.92% (130771/233872)

@yiguolei
yiguolei merged commit d084ff1 into apache:branch-4.1 Aug 31, 2026
36 of 39 checks passed
gavinchou pushed a commit that referenced this pull request Aug 31, 2026
Related PR: #67293

Problem Summary:

`CachedRemoteFileReader::prefetch_range()` submits fire-and-forget
dry-run tasks. A running task keeps the cached reader and its underlying
`HdfsFileReader` alive, but it does not keep the query `RuntimeProfile`
alive. Because `HdfsFileReader` stored raw pointers to query-profile
timers and counters, a delayed prefetch read could update them after the
query profile had been destroyed and trigger a use-after-free.

Remove the query `RuntimeProfile` dependency and HDFS-specific
query-profile timers and counters from `HdfsFileReader`. This also
removes the now-unused profile passthrough from `HdfsFileSystem` and
updates all affected call sites. Generic file-reader and file-cache
statistics plus process-wide HDFS bvars remain unchanged; HDFS reads and
cache behavior are unchanged.

The forward-port conflict in `hdfs_file_reader.cpp` was resolved by
preserving master's WorkloadGroup/ResourceContext includes and master's
removal of the old `compile_check_begin/end` pair while applying
#67293's profile-removal semantics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants