Skip to content

fix: Improve validation for system shared memory register#8336

Merged
pskiran1 merged 3 commits into
mainfrom
spolisetty_pb_shm_fix
Aug 11, 2025
Merged

fix: Improve validation for system shared memory register#8336
pskiran1 merged 3 commits into
mainfrom
spolisetty_pb_shm_fix

Conversation

@pskiran1

@pskiran1 pskiran1 commented Aug 8, 2025

Copy link
Copy Markdown
Member

What does the PR do?

This PR is an enhancement for #8273.
Previously, users could bypass validation by using a leading slash in shm_key, such as "/triton_python_backend_shm_region_my_test_shm_key", "///triton_python_backend_shm_region_my_test_shm_key"

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:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

Where should the reviewer start?

Test plan:

  • CI Pipeline ID: 33011596

Caveats:

Background

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

  • closes GitHub issue: #xxx

@pskiran1 pskiran1 added the PR: fix A bug fix label Aug 8, 2025
Comment thread src/common.cc
const std::size_t first_non_slash = key_view.find_first_not_of('/');

// If the entire key is slashes
if (first_non_slash == std::string_view::npos) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This validation is optional; if we skip it, the code will fail later.
Since we need to check for slashes in this function, I added this validation to return error early.

@pskiran1
pskiran1 marked this pull request as ready for review August 8, 2025 17:06
@pskiran1
pskiran1 requested review from tanmayv25 and yinggeh August 8, 2025 17:25

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

Left comment

@yinggeh
yinggeh self-requested a review August 8, 2025 18:06
@yinggeh

yinggeh commented Aug 8, 2025

Copy link
Copy Markdown
Contributor

Is ./triton_python_backend_shm_region_my_test_shm_keyanother corner case?

@pskiran1

Copy link
Copy Markdown
Member Author

Is ./triton_python_backend_shm_region_my_test_shm_keyanother corner case?

Thank you for your note. I verified it, and it’s not an issue. It bypasses the validation, but the server still rejects it because it does not read the PB SHM and treats it as a different one.

# curl -X POST "localhost:8000/v2/systemsharedmemory/region/mytestmem/register" -H "Content-Type: application/json" -d '{"key": "./triton_python_backend_shm_region_858ece88-9702-44ea-b1e7-5c4ee9258c1f", "byte_size":100000}' | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   224  100   122  100   102   120k   100k --:--:-- --:--:-- --:--:--  218k
{
  "error": "Unable to open shared memory region: './triton_python_backend_shm_region_858ece88-9702-44ea-b1e7-5c4ee9258c1f'"
}

If we try to create an shm handle using the client, POSIX shm_open also rejects it with an error.

File "/usr/local/lib/python3.12/dist-packages/tritonclient/utils/shared_memory/__init__.py", line 82, in create_shared_memory_region
    shm_handle._mpsm_handle = mpshm.SharedMemory(shm_key)
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/multiprocessing/shared_memory.py", line 104, in __init__
    self._fd = _posixshmem.shm_open(
               ^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 22] Invalid argument: '/./triton_python_backend_shm_region_faa7f1c0-4b33-47a5-a893-d1b3e5178ad4

@pskiran1
pskiran1 merged commit 21245cf into main Aug 11, 2025
3 checks passed
@pskiran1
pskiran1 deleted the spolisetty_pb_shm_fix branch August 11, 2025 05:17
mc-nv added a commit that referenced this pull request Aug 14, 2025
@pskiran1 pskiran1 mentioned this pull request Aug 14, 2025
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fix A bug fix

Development

Successfully merging this pull request may close these issues.

2 participants