nvidia-drm: reject out-of-bounds semaphore surface indices - #1320
Open
VaggelisGian wants to merge 1 commit into
Open
nvidia-drm: reject out-of-bounds semaphore surface indices#1320VaggelisGian wants to merge 1 commit into
VaggelisGian wants to merge 1 commit into
Conversation
DRM_IOCTL_NVIDIA_SEMSURF_FENCE_CTX_CREATE is reachable from unprivileged render node clients and passes a 64-bit semaphore index that __nv_drm_semsurf_fence_ctx_new() used unchecked to shift the CPU mappings of the imported surface by index * stride. NVKMS maps exactly the surface size reported by the import parameters, but never exposed that size to nvidia-drm, so there was nothing to validate against. A later fence operation then reads the semaphore and max-submitted values through the shifted pointers, so any index at or beyond the end of the mapped surface makes the kernel read outside of it. importSemaphoreSurface() now returns the mapped size through a new out parameter, and the context constructor rejects indices outside the surface, indices above NV_U32_MAX which the RM semaphore index field cannot represent, and layouts whose max-submitted offset does not fit in one stride, before any pointer arithmetic happens. Test Plan: No build or runtime test possible in the preparation environment (no Linux kernel toolchain, no NVIDIA GPU). Verified statically: both KAPI header copies updated identically; nvKmsKapiImportSemaphoreSurface prototype and implementation agree with the table assignment; validation runs after a successful import and before the first shifted dereference; the rejection path frees the imported surface via failed_alloc_fence_context; stride == 0 is rejected before the division so unsupported configurations cannot divide by zero.
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
1 participant
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.
Problem
DRM_IOCTL_NVIDIA_SEMSURF_FENCE_CTX_CREATE(available withDRM_RENDER_ALLOW)takes a 64-bit semaphore surface index from userspace and uses it unchecked as
an offset into the imported semaphore surface.
nv_drm_semsurf_fence_ctx_createpasses the index straight to
__nv_drm_get_semsurf_ctx_seqno, which computesthe slot address from the mapped base pointer plus the shifted index without
any bound against the real surface size. An unprivileged client can therefore
pass any 64-bit index and cause an out-of-bounds read of kernel-mapped memory
at context creation time; the garbage value then becomes the fence's sequence
number.
The wait-side ioctl validates its inputs (#1297 covered a related failure
there), but the context-creation path never bounds the index.
Change
Two parts:
importSemaphoreSurfacecall gains anNvU64 *pSurfaceSizeout-parameter so the DRM layer learns the real surface geometry. Both copies
of the header (
kernel-open/common/inc/nvkms-kapi.h,src/nvidia-modeset/kapi/interface/nvkms-kapi.h) are updated identically,together with the internal declaration and the
nvkms-kapi-sync.cimplementation. This follows existing KAPI signature-change precedent in
this tree (b5bf85a added the pMaxSubmittedMap parameters to the same
interface).
pointer arithmetic:
per-slot tail read stays inside the slot
On constructor failure the imported surface is released exactly once by
jumping to the allocation-failure label instead of the plain failure label,
which previously leaked it while the caller-side cleanup re-freed state.
Test Plan
Static verification only. This machine is a Windows host with no Linux
toolchain, no kernel headers, and no NVIDIA GPU, so neither
make modulesnor a runtime reproduction was possible here. Review performed:
importSemaphoreSurfaceupdated for the new parameternvkms-kapi.hcopies byte-identicalDriver tree version: 610.57.04. Hardware/kernel reproduction will follow if
maintainers want one; the code path is reachable from any process with access
to a render node.