Skip to content

Conversation

yinggeh
Copy link
Contributor

@yinggeh yinggeh commented Jul 1, 2025

What does the PR do?

When attacker registers the same shm created by python backend, they can overwrite MessageQueueShm::head data with a very large index and inject malicious code to the memory space.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

  • fix

Related PRs:

Where should the reviewer start?

The index boundary check is in src/message_queue.h

Test plan:

  • CI Pipeline ID:
    30975011

Caveats:

Background

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

@yinggeh yinggeh self-assigned this Jul 1, 2025
@yinggeh yinggeh added the bug Something isn't working label Jul 1, 2025
@yinggeh yinggeh changed the title fix: Additional check on message queue indices fix: Additional check on message queue shm indices Jul 1, 2025
@yinggeh yinggeh changed the title fix: Additional check on message queue shm indices fix: MessageQueueShm head index boundary check Jul 1, 2025
@yinggeh
Copy link
Contributor Author

yinggeh commented Jul 1, 2025

No unit test because triton-inference-server/server#8273 makes the exploitation impossible.

@yinggeh yinggeh requested review from kthui and krishung5 July 2, 2025 00:57
@yinggeh yinggeh marked this pull request as draft July 2, 2025 18:00
@yinggeh yinggeh marked this pull request as ready for review July 2, 2025 23:42
@yinggeh yinggeh requested a review from Tabrizian July 2, 2025 23:47
Copy link
Member

@pskiran1 pskiran1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Will wait for other expert review.

@statiraju
Copy link

@tanmayv25 can you help approve.

@tanmayv25
Copy link
Contributor

LGTM

@yinggeh yinggeh merged commit 8cfdffe into main Jul 7, 2025
3 checks passed
mc-nv added a commit that referenced this pull request Jul 23, 2025
@yinggeh yinggeh deleted the yinggeh-DLIS-8378-check-message-queue-boundary branch July 28, 2025 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working DLIS-8378 TPRD-1628
Development

Successfully merging this pull request may close these issues.

5 participants