linux: remove sudo escalation from probe_driver and fix CI test setup - #370
linux: remove sudo escalation from probe_driver and fix CI test setup#370cshilwant wants to merge 2 commits into
Conversation
A library must not attempt to escalate its own privileges.
The sudo modprobe fallback was incorrect for three reasons:
1. A library silently attempting privilege escalation on behalf of its
caller is unexpected behavior and a security concern.
2. If modprobe failed due to missing CAP_SYS_MODULE, sudo will also
fail unless passwordless sudo is explicitly configured in sudoers
making the fallback redundant in practice.
3. On embedded and production Linux systems, sudo is often not
installed, causing system() to fail silently or hang waiting for
a password prompt.
If the driver is not found after modprobe, return -ENODEV and let the
caller handle it.
Fixes: commit 7bda1db ("Initial commit")
Signed-off-by: Chirag Shilwant <c-shilwant@ti.com>
The nr_tests job was relying on libmetal's internal sudo modprobe fallback in metal_linux_probe_driver() to load UIO modules at runtime. Now that the sudo escalation has been removed from the library, the CI environment must explicitly load the required modules before invoking the test suite. Add a "sudo modprobe" step for uio, uio_pdrv_genirq, and uio_dmem_genirq prior to make test. Module loading with sudo privileges is the responsibility of the environment, not the library. Signed-off-by: Chirag Shilwant <c-shilwant@ti.com>
cshilwant
changed the title
arnopo
left a comment
arnopo
left a comment
There was a problem hiding this comment.
Tested with success on my PC,
Just a comment to request an extra fix of an issue i found during my test. Not mandatory, I can address it later
| - name: execute test | ||
| run: | | ||
| sudo modprobe uio uio_pdrv_genirq uio_dmem_genirq | ||
| cd build |
There was a problem hiding this comment.
Could you take opportunity to remove the cd build that is already done during build step ?
There was a problem hiding this comment.
Each run keyword represents a new process and shell in the runner environment
@arnopo Thanks for the suggestion! As per the GitHub Actions workflow syntax reference (jobs.<job_id>.steps[*].run - https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun), doesn't each run: step spawn a new shell process, meaning the cd build from the build step would not carry over to the execute test step?
Probably also the reason why the test logs step references the full path build/Testing/Temporary/LastTest.log - https://github.com/OpenAMP/libmetal/blob/main/.github/workflows/continuous-integration.yml#L76 rather than just Testing/Temporary/LastTest.log ?
4 participants
Removes the
sudo modprobefallback frommetal_linux_probe_driver()in
lib/system/linux/device.c. A library must not silently attemptprivilege escalation on behalf of its caller - this is a security
concern and fails on embedded/production systems where
sudois absent or not configured for password-less use.
Updates the CI nonreg test workflow to explicitly load the required
UIO kernel modules (
uio,uio_pdrv_genirq,uio_dmem_genirq)before running
make test, since the environment (& not the library)is responsible for module loading