Update nccl test script to fix enroot directory issue in A3H#5324
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical "Permission denied" error that prevented NCCL tests from running successfully on A3H machines. The fix ensures that the enroot container runtime correctly identifies and uses a user-specific, writable directory for its temporary files, thereby bypassing the previous issue where it attempted to write to a restricted system path. This change improves the reliability of NCCL test execution in environments where XDG_RUNTIME_DIR might not be pre-configured. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses a permission issue with enroot by setting the XDG_RUNTIME_DIR environment variable. However, the current implementation introduces a security vulnerability: the fallback mechanism creates a temporary directory in /tmp with a predictable name without proper ownership checks or error handling. This makes it susceptible to symlink attacks, allowing another user to pre-create the directory and potentially access sensitive runtime files. A suggested fix using mktemp -d has been provided in a review comment to create the directory securely and simplify the logic. This approach should also be considered for other similar scripts in the repository.
examples/machine-learning/a3-highgpu-8g/nccl-tests/import_pytorch_container.sh
Show resolved
Hide resolved
1225b41 to
b52bf02
Compare
|
These changes are working as intended in local build. |
db31f62
into
GoogleCloudPlatform:develop
This PR resolves the error encountered while running NCCL test for a3h machines:
Error encountered in the NCCL tests:
mkdir: cannot create directory ‘/run/enroot’: Permission deniedFix:
XDG_RUNTIME_DIRis a standard Linux environment variable defined by the XDG Base Directory Specification. It points to a directory specific to the logged-in user for storing small, temporary runtime files. Enroot uses this variable to determine where to create its workspace. If it's unset, Enroot falls back to a hardcoded system path (/run) which causes the crash.We are resolving the Permission denied error by manually setting the
XDG_RUNTIME_DIRenvironment variable to a user-specific, world-writable directory in /tmp (e.g., /tmp/enroot-runtime-$(id -u)). This ensures enroot uses a safe, writable workspace instead of defaulting to the restricted, root-owned /run/enroot pathSubmission Checklist
NOTE: Community submissions can take up to 2 weeks to be reviewed.
Please take the following actions before submitting this pull request.