fix(cloudxr): LD_PRELOAD bundled OpenSSL into the runtime worker#728
fix(cloudxr): LD_PRELOAD bundled OpenSSL into the runtime worker#728johnnynunez wants to merge 1 commit into
Conversation
The runtime worker imports asyncio (via isaacteleop.cloudxr.runtime), which imports Python's ssl module and loads the SYSTEM OpenSSL before the native streaming stack dlopens its bundled libssl_nvst/libcrypto_nvst. Two OpenSSL builds resolving symbols in one process SIGSEGV inside DtlsTransport::initDtlsContext -> SSL_CTX_use_certificate exactly when the client's WebRTC DTLS handshake starts - i.e. the runtime crashes the moment a headset connects, while idle waiting works fine. Spawn the worker with LD_PRELOAD pointing at the bundled OpenSSL libraries (preserving any pre-existing LD_PRELOAD, and only when the bundled files exist) so every OpenSSL symbol binds to the single build libNvStreamServer.so was compiled against. Signed-off-by: Johnny <johnnync13@gmail.com>
|
📝 Docs preview is not auto-deployed for fork PRs. A maintainer with write access to |
📝 WalkthroughWalkthroughThe CloudXRLauncher's subprocess launch logic was updated to construct a dedicated worker_env dictionary instead of directly using a copy of the OS environment. When bundled OpenSSL libraries (libcrypto_nvst.so.3, libssl_nvst.so.3) exist under the launcher's native directory, their paths are prepended to LD_PRELOAD, preserving any pre-existing value. The runtime subprocess is then started using this modified environment. Estimated code review effort: 2 (Simple) | ~10 minutes Security NotePrepending to LD_PRELOAD affects dynamic library resolution for the spawned subprocess; verify that the bundled library paths are validated against tampering (e.g., not attacker-writable) before existence checks, since LD_PRELOAD injection from a writable location could allow code execution in the subprocess's context. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/cloudxr/python/launcher.py`:
- Around line 166-169: LD_PRELOAD cannot safely represent bundled OpenSSL paths
that contain spaces, so the current preload assembly in launcher.py is not
enough for installations under space-containing directories. Update the logic
around the bundled_ssl/LD_PRELOAD handling to either avoid preloading when any
library path contains spaces or switch to a different loading strategy that does
not rely on LD_PRELOAD path parsing. Keep the fix localized to the worker_env
setup in the launcher flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 906bea28-761f-4f1e-a7cc-defd1e8f8874
📒 Files selected for processing (1)
src/core/cloudxr/python/launcher.py
| if all(os.path.isfile(lib) for lib in bundled_ssl): | ||
| preload = " ".join(bundled_ssl) | ||
| prev = worker_env.get("LD_PRELOAD") | ||
| worker_env["LD_PRELOAD"] = f"{preload} {prev}" if prev else preload |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of the file.
cat -n src/core/cloudxr/python/launcher.py | sed -n '145,185p'Repository: NVIDIA/IsaacTeleop
Length of output: 2414
🌐 Web query:
glibc LD_PRELOAD spaces colons separator documentation
💡 Result:
According to the official glibc documentation (specifically the ld.so(8) manual page), the LD_PRELOAD environment variable accepts a list of ELF shared objects to be preloaded [1][2]. The items in this list can be delimited by either spaces or colons [3][2]. Crucially, there is no support for escaping either the space or colon characters [2][4]. As a result, if a file path contains a space, it cannot be correctly specified within the LD_PRELOAD list because the dynamic linker will interpret the space as a delimiter, splitting the path into multiple invalid segments [5][2]. The /etc/ld.so.preload file, which is also used for preloading, likewise expects a whitespace-separated list [6][4]. When both LD_PRELOAD and /etc/ld.so.preload are used, the libraries specified in LD_PRELOAD are loaded first [6][4].
Citations:
- 1: https://man7.org/linux/man-pages/man8/ld.so.8.html
- 2: https://manpages.debian.org/bookworm/manpages/ld.so.8.en.html
- 3: https://manpages.debian.org/bullseye/manpages/ld-linux.so.8.en.html
- 4: https://manpages.debian.org/buster/manpages/ld-linux.so.8.en.html
- 5: https://unix.stackexchange.com/questions/796527/how-to-specify-elf-shared-objects-in-ld-preload-if-their-path-contains-a-space
- 6: https://man.archlinux.org/man/ld.so.8.en.raw
LD_PRELOAD can’t encode paths with spaces. Spaces and colons are both separators here, so changing the joiner won’t help for installs under space-containing directories; if those installs are supported, the bundled OpenSSL needs a space-free preload path or a different loading strategy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/cloudxr/python/launcher.py` around lines 166 - 169, LD_PRELOAD
cannot safely represent bundled OpenSSL paths that contain spaces, so the
current preload assembly in launcher.py is not enough for installations under
space-containing directories. Update the logic around the bundled_ssl/LD_PRELOAD
handling to either avoid preloading when any library path contains spaces or
switch to a different loading strategy that does not rely on LD_PRELOAD path
parsing. Keep the fix localized to the worker_env setup in the launcher flow.
1 participant
Fix ARM cloudXR
Summary by CodeRabbit