Skip to content

fix: remove dangling TraceContext pointer storage in SealedIndexTranslator (#48494)#48496

Open
chyezh wants to merge 1 commit intomilvus-io:2.6from
chyezh:fix/48494-sealed-index-translator-trace-ctx-uaf-2.6
Open

fix: remove dangling TraceContext pointer storage in SealedIndexTranslator (#48494)#48496
chyezh wants to merge 1 commit intomilvus-io:2.6from
chyezh:fix/48494-sealed-index-translator-trace-ctx-uaf-2.6

Conversation

@chyezh
Copy link
Copy Markdown
Contributor

@chyezh chyezh commented Mar 24, 2026

SealedIndexTranslator stored a TraceContext member containing raw traceID/spanID pointers originating from Go memory. These pointers dangle when get_cells() is called during async warmup or cache re-population, causing heap-buffer-overflow detected by ASAN.

Remove the TraceContext member and use an empty TraceContext in get_cells() instead, since the original request's trace context is meaningless for deferred index loading.

issue: #48494
pr: #48495

@chyezh chyezh added this to the 2.6.14 milestone Mar 24, 2026
@sre-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chyezh
To complete the pull request process, please assign xiaofan-luan after the PR has been reviewed.
You can assign the PR to them by writing /assign @xiaofan-luan in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot added the size/S Denotes a PR that changes 10-29 lines. label Mar 24, 2026
@mergify mergify bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Mar 24, 2026
@sre-ci-robot sre-ci-robot added the do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first label Mar 24, 2026
@sre-ci-robot
Copy link
Copy Markdown
Contributor

[INFO] PR Label Summary by Default
[FAILED] PR #48495 not merged

Use /refresh-label to update related check and label manually

@sre-ci-robot
Copy link
Copy Markdown
Contributor

[ci-v2-notice]
Notice: New ci-v2 system is enabled for this PR.

To rerun ci-v2 checks, comment with:

  • /ci-rerun-code-check // for ci-v2/code-check
  • /ci-rerun-build // for ci-v2/build
  • /ci-rerun-build-all // for ci-v2/build-all (multi-arch builds)
  • /ci-rerun-buildenv // for ci-v2/build-env (build milvus-env builder images)
  • /ci-rerun-ut-integration // for ci-v2/ut-integration, will rerun ci-v2/build
  • /ci-rerun-ut-go // for ci-v2/ut-go, will rerun ci-v2/build
  • /ci-rerun-ut-cpp // for ci-v2/ut-cpp
  • /ci-rerun-ut // for all ci-v2/ut-integration, ci-v2/ut-go, ci-v2/ut-cpp, will rerun ci-v2/build
  • /ci-rerun-e2e-arm // for ci-v2/e2e-arm
  • /ci-rerun-e2e-default // for ci-v2/e2e-default
  • /ci-rerun-ciloop // for ci-v2/ciloop (build + unit tests in one pipeline)
  • /ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests, AMD)
  • /ci-rerun-gosdk-arm // for ci-v2/go-sdk-arm (Go SDK E2E tests, ARM)

If you have any questions or requests, please contact @zhikunyao.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.25%. Comparing base (cb08db0) to head (80845f5).
⚠️ Report is 734 commits behind head on 2.6.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              2.6   #48496      +/-   ##
==========================================
+ Coverage   76.99%   77.25%   +0.25%     
==========================================
  Files        1700     1960     +260     
  Lines      262533   309563   +47030     
==========================================
+ Hits       202142   239155   +37013     
- Misses      53550    62857    +9307     
- Partials     6841     7551     +710     
Components Coverage Δ
Client 79.48% <78.30%> (+1.33%) ⬆️
Core 83.39% <93.03%> (+1.18%) ⬆️
Go 75.85% <52.12%> (+0.46%) ⬆️
see 1275 files with indirect coverage changes
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sre-ci-robot sre-ci-robot added low-code-coverage add test-label from zhikun, diff coverage > 80% and removed low-code-coverage add test-label from zhikun, diff coverage > 80% labels Mar 24, 2026
@mergify mergify bot added the ci-passed label Mar 24, 2026
…I mismatch

milvus-common defines OPENTELEMETRY_STL_VERSION=2017 (PUBLIC), which
makes nostd::shared_ptr an alias for std::shared_ptr (16 bytes). However,
Milvus core OBJECT libraries (segcore, index, query, etc.) never receive
this define because CMake TARGET_OBJECTS does not propagate transitive
compile definitions from link targets.

Without the define, these translation units use the custom nostd::shared_ptr
implementation with a 32-byte PlacementBuffer — a completely different
memory layout. This ODR violation causes heap-buffer-overflow detected by
ASAN when OTel spans are created/copied across the ABI boundary (e.g.
during async cache warmup in SealedIndexTranslator::get_cells).

Fix by adding add_compile_definitions(OPENTELEMETRY_STL_VERSION=2017) at
the top-level CMakeLists.txt so all translation units use the same layout.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: chyezh <chyezh@outlook.com>
@chyezh chyezh force-pushed the fix/48494-sealed-index-translator-trace-ctx-uaf-2.6 branch from e578e57 to 80845f5 Compare March 26, 2026 08:58
@sre-ci-robot sre-ci-robot added area/compilation size/XS Denotes a PR that changes 0-9 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Mar 26, 2026
@sre-ci-robot
Copy link
Copy Markdown
Contributor

[INFO] PR Label Summary by Default
[FAILED] PR #48495 not merged

Use /refresh-label to update related check and label manually

@sre-ci-robot sre-ci-robot added low-code-coverage add test-label from zhikun, diff coverage > 80% and removed ci-passed labels Mar 26, 2026
@zhikunyao
Copy link
Copy Markdown
Collaborator

/ci-rerun-gosdk

@sre-ci-robot sre-ci-robot removed the low-code-coverage add test-label from zhikun, diff coverage > 80% label Mar 26, 2026
@mergify mergify bot added the ci-passed label Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compilation ci-passed dco-passed DCO check passed. do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first kind/bug Issues or changes related a bug size/XS Denotes a PR that changes 0-9 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants