SDLe fix: refactor, remove drop_privileges() and capability management from main.c#9
Merged
Conversation
dvledtx delegates all VFIO and hugepage operations to mtl_init() inside the MTL library. Whether those operations require elevated privileges is purely a system configuration concern, not an application concern. With the recommended pre-configuration on the host: - udev rule granting the service user ownership of /dev/vfio/<group> - LimitMEMLOCK=infinity in the systemd unit (or /etc/security/limits.conf) mtl_init() succeeds without any elevated capabilities. The application should run as a plain unprivileged service user with no capability management needed. Removes: - drop_privileges() function and its E-1 comment block - #include <linux/capability.h> - #include <sys/prctl.h> - #include <sys/syscall.h> - #include <sys/mman.h> (unused after removal)
dvledtx delegates all VFIO and hugepage operations to mtl_init() inside the MTL library. Whether those operations require elevated privileges is purely a system configuration concern, not an application concern. With the recommended pre-configuration on the host: - udev rule granting the service user ownership of /dev/vfio/<group> - LimitMEMLOCK=infinity in the systemd unit (or /etc/security/limits.conf) mtl_init() succeeds without any elevated capabilities. The application should run as a plain unprivileged service user with no capability management needed. Removes: - drop_privileges() function and its E-1 comment block - #include <linux/capability.h> - #include <sys/prctl.h> - #include <sys/syscall.h> - #include <sys/mman.h> (unused after removal)
5173804 to
cc9d191
Compare
…ualCloud/directview-led-software-toolkit into remove-privileged-access
…om running binary
…ile/ldd/nm instead
…6-64 for runner portability
- Remove /tmp/ from ALLOWED_LOG_PREFIXES in main.c (world-writable, symlink attack risk) - Add test_validate_log_path_rejects_tmp: verify /tmp paths are rejected - Add test_validate_log_path_allows_var_log: verify /var/log/ is allowed - Add test_validate_log_path_allows_cwd_relative: verify cwd-relative OK - Add test_not_running_as_sudo: verify tests are not run as root
- Add lstat() check in main.c to reject config files that are symlinks - Add test_main_rejects_symlinked_config unit test to verify rejection
roshan-ku
approved these changes
May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dvledtx delegates all VFIO and hugepage operations to mtl_init() inside the MTL library. Whether those operations require elevated privileges is purely a system configuration concern, not an application concern.
With the recommended pre-configuration on the host:
mtl_init() succeeds without any elevated capabilities. The application should run as a plain unprivileged service user with no capability management needed.
Removes:
Description
Checklist
Code Quality
Testing
Review Readiness
Documentation
Security
PR Type
What kind of change does this PR introduce?