Skip to content

nvidia-modeset: ignore nested nvRevokeDevice() calls - #1395

Open
SammyTourani wants to merge 1 commit into
NVIDIA:mainfrom
SammyTourani:fix/issue-1284
Open

SammyTourani wants to merge 1 commit into
NVIDIA:mainfrom
SammyTourani:fix/issue-1284

Conversation

@SammyTourani

Copy link
Copy Markdown

Fixes #1284

nvRevokeDevice() now returns early when it is called while a revoke is already running.
A file-scope static NvBool revokeInProgress tracks this, following the
static int suspendCounter pattern already used for nested suspend/resume in the same
file. The flag is set before the loop over perOpenIoctlList and cleared after it.

The nested call can only come from FreeDeviceReference() -> ReleaseModesetOwnership()
-> RestoreConsole() when the core channel cannot be reallocated. At that point the outer
loop is already freeing every client's reference. The nested pass adds only one thing: a
second FreeDeviceReference() for the pOpenDev that is currently being freed. With the
early return, each client reference is dropped exactly once and the device is freed after
the last one, as the outer revoke intends. Revokes that are not nested behave as before.
Examples are a resume failure with no modeset owner, and RestoreConsole() failing after
NVKMS_IOCTL_RELEASE_OWNERSHIP.

One file, +19 lines (src/nvidia-modeset/src/nvkms.c).

Scope:

  • This does not address the GSP/RM failures that trigger the path (Xid 119/120/154, "GSP
    unload failed"). The GPU still needs recovery afterwards, but suspend now fails without
    a kernel oops and a use-after-free.
  • The same double FreeDeviceReference() can still be reached with no outer revoke:
    nvKmsClose()/NVKMS_IOCTL_FREE_DEVICE of the modeset owner while the core channel
    cannot be reallocated (for example, Xorg exiting after the GPU died while other clients
    still hold the device). I left that path alone. Fixing it needs FreeDeviceReference()
    to keep its own device reference until after ReleaseModesetOwnership(), which changes
    the ordering of the normal close path.

Verification

The repository has no test suite, and the bug needs a failing GSP. So I verified with (1) a
userspace reproduction that runs the unmodified nvidia-modeset code, and (2) a compile of
the changed file with the Makefile's exact flags for x86_64 Linux.

1. Userspace reproduction. The harness compiles the real nvkms.c, nvkms-evo.c,
nvkms-rm.c, nvkms-utils.c and nvkms-vblank-sem-control.c with clang and
AddressSanitizer. It provides:

  • the nvkms_* OS interface on libc;
  • an RM API that succeeds until "the GPU dies" and then returns NV_ERR_TIMEOUT for every call;
  • a device in the state nvAllocDevEvo()/nvAllocCoreChannelEvo() leave it in;
  • return 0 stubs for the ~136 functions outside those files that these paths never depend on.

Clients are set up through the real NVKMS_IOCTL_ALLOC_DEVICE, GRAB_OWNERSHIP and
ENABLE_VBLANK_SEM_CONTROL ioctls. Then the real nvKmsSuspend() runs, the GPU "dies",
and the real nvKmsResume() runs, which is what nv_suspend_devices() does when RM
suspend fails. A second build ("stale") keeps freed memory mapped with its old contents,
like a vfree()d buffer read through a stale TLB entry, so the run goes all the way to the
field crash instead of stopping at the first ASan report.

Command: for m in asan stale; do ./build.sh $BASE out-base-$m $m; ./build.sh $FIX out-fix-$m $m; done; ./run-matrix.sh
(run in the harness directory; $BASE = pristine 615.71.09 tree, $FIX = this branch)

Result: 28 runs (7 scenarios x 2 trees x 2 builds). The repository has no test suite of
its own, so these are the only runs.

scenario    build  mode   result
field       base   asan   ASAN: heap-use-after-free
field       base   stale  SIGSEGV at 0x2740
field       fix    asan   ok (device freed=1, live nvkms allocations=0)
field       fix    stale  ok (device freed=1, live nvkms allocations=0)
userfirst   base   asan   ok (device freed=1, live nvkms allocations=0)
userfirst   base   stale  ok (device freed=1, live nvkms allocations=0)
userfirst   fix    asan   ok (device freed=1, live nvkms allocations=0)
userfirst   fix    stale  ok (device freed=1, live nvkms allocations=0)
twousers    base   asan   ASAN: heap-use-after-free
twousers    base   stale  SIGSEGV at 0x2740
twousers    fix    asan   ok (device freed=1, live nvkms allocations=0)
twousers    fix    stale  ok (device freed=1, live nvkms allocations=0)
xorg        base   asan   ASAN: heap-use-after-free
xorg        base   stale  SIGSEGV at 0x2740
xorg        fix    asan   ok (device freed=1, live nvkms allocations=0)
xorg        fix    stale  ok (device freed=1, live nvkms allocations=0)
noowner     base   asan   ok (device freed=1, live nvkms allocations=0)
noowner     base   stale  ok (device freed=1, live nvkms allocations=0)
noowner     fix    asan   ok (device freed=1, live nvkms allocations=0)
noowner     fix    stale  ok (device freed=1, live nvkms allocations=0)
release     base   asan   ok (device freed=1, live nvkms allocations=0)
release     base   stale  ok (device freed=1, live nvkms allocations=0)
release     fix    asan   ok (device freed=1, live nvkms allocations=0)
release     fix    stale  ok (device freed=1, live nvkms allocations=0)
closeowner  base   asan   ASAN: heap-use-after-free
closeowner  base   stale  SIGSEGV at 0x2740
closeowner  fix    asan   ASAN: heap-use-after-free
closeowner  fix    stale  SIGSEGV at 0x2740

Scenarios:

  • field: kernel client owns modeset (nvidia-drm, fbdev=1), then a user client with a vblank semaphore control.
  • twousers: the owner plus two such user clients.
  • xorg: nvidia-drm without ownership, a user-space owner, and a user client.
  • userfirst: the user client opened before the owner.
  • noowner: no modeset owner.
  • release: RELEASE_OWNERSHIP while the GPU is dead (a revoke that is not nested).
  • closeowner: the owner closes its fd while the GPU is dead (the path this patch leaves
    alone, see above).

Unpatched field, symbolized ASan report (the freed region is the 10592-byte NVDevEvoRec):

ERROR: AddressSanitizer: heap-use-after-free ... READ of size 8
    #0 RestoreConsole (nvkms.c:1125)            <- nested nvRevokeDevice() (inlined)
    #1 FreeDeviceReference (nvkms.c:1649)       <- owner: ReleaseModesetOwnership()
    #2 nvRevokeDevice (nvkms.c)
    #3 nvResumeDevEvo (nvkms-evo.c:5512)
    #4 nvKmsResume (nvkms.c:7016)
freed by thread T0 here:
    #1 nvFreeDevEvo (nvkms-evo.c:8800)
    #2 FreeDeviceReference (nvkms.c:1643)       <- owner freed a second time
    #3 RestoreConsole (nvkms.c:1125)
    #4 FreeDeviceReference (nvkms.c:1649)
    #5 nvRevokeDevice ...

Unpatched field, stale build:
SIGSEGV, fault address 0x2740, pc -> nvEvoDisableVblankSemControl (nvkms-vblank-sem-control.c:236).
0x2740 = offsetof(NVDispEvoRec, vblankApiHeadState[0].vblankCount) in 615.71.09, which
is the field crash.

2. Compile check with the project's flags.

Command: I took the src/nvkms.c compile line printed by
make -n -C src/nvidia-modeset TARGET_ARCH=x86_64 CC=clang
(-mcmodel=kernel -mno-red-zone -msoft-float -ffreestanding -Wall -Wextra ... -std=gnu11 -c src/nvkms.c)
and ran it unchanged except for clang --target=x86_64-linux-gnu. I ran it in
src/nvidia-modeset of the pristine tree and of this branch.

Result:

base exit=0, 0 warnings
fix  exit=0, 0 warnings   (ELF 64-bit LSB relocatable, x86-64)

Not done: I did not run the patched driver on real hardware or build it with kbuild
against a kernel tree (no NVIDIA GPU or Linux kernel headers here).

If the core channel cannot be reallocated on resume, nvResumeDevEvo()
calls nvRevokeDevice(), which calls FreeDeviceReference() for every
client.  For the modeset owner, FreeDeviceReference() releases ownership,
RestoreConsole() fails to reallocate the core channel again and calls
nvRevokeDevice() a second time.  The nested call runs
FreeDeviceReference() again for the owner, so its device reference is
dropped twice and the device is freed while other clients still point
at it.  Their teardown then reads the freed NVDevEvoRec, which oopses in
nvEvoDisableVblankSemControl() with a NULL pDispEvo.

Return early from a nested nvRevokeDevice(); the outer loop already
frees every remaining reference.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

tdortman added a commit to tdortman/dotfiles that referenced this pull request Sep 29, 2026
A failed resume makes `nvidia-modeset` call `nvRevokeDevice` from
inside itself. That frees the device twice and oopses in the PM
notifier, which wedges the machine.

Apply `NVIDIA/open-gpu-kernel-modules#1395` (fixes #1284) to the open
module, pinned to its commit. Drop it once the PR is merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants