Skip to content

nvidia: free the GSP TSG when channel group construction fails after the alloc RPC - #1398

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

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

Conversation

@SammyTourani

Copy link
Copy Markdown

Fixes #1379

In kchangrpapiConstruct_IMPL(), record that the alloc RPC succeeded (bRpcAllocated, the same way kchannelConstruct_IMPL()
tracks its channel RPC). In the failure path, free the object on GSP-RM (or the vGPU host) with NV_RM_RPC_FREE:

  • The free runs before kchangrpDestroy() releases the grpID on the kernel-RM side, which is the same order as a normal free
    (RPC free, then destructor).
  • The failure path always holds the GPU lock here. The ctxBufPoolReserve() path re-acquires it before goto failed.
  • An allocation whose RPC itself failed does not set the flag, so nothing is freed on GSP-RM in that case. For a duplicate
    handle, that object belongs to someone else.
  • I did not use NV_RM_RPC_FREE_ON_ERROR because it assigns to a variable named status, which this function does not have.
    NV_RM_RPC_FREE with a local status is the pattern video_mem.c uses in its constructor failure path.

One file, +17 lines.

Verification

There was no NVIDIA GPU available, so this is not tested on hardware.

I used a userspace harness that compiles these unmodified driver sources natively, with the RM include paths and defines from
src/nvidia/Makefile:

  • kernel_channel_group_api.c;
  • g_kernel_channel_group_api_nvoc.c, the NVOC objCreate/ctor that resservResourceFactory() goes through;
  • kernel_fifo.c, for kfifoChidMgrAllocChannelGroupHwID() and kfifoChidMgrFreeChannelGroupHwID();
  • base_utils.c, nvassert.c and nvstatus.c.

Everything else is faked:

  • a GA104 OBJGPU that is a GSP client;
  • kchangrpInit/kchangrpDestroy, reduced to their grpID handling on the real allocator;
  • GSP-RM, modelled as a per-client handle table plus a second CHID_MGR. That CHID_MGR is driven by the real
    kfifoChidMgrAllocChannelGroupHwID() with the grpID from internalFlags.
  • ctxBufPoolReserve(), which returns NV_ERR_NO_MEMORY on demand.

Any other external symbol aborts if it is called (135 stubs, none hit). The build uses AddressSanitizer, and every GSP-RM RPC
must be issued with the GPU lock held.

The scenario:

  1. Allocate 10 TSGs.
  2. With vidmem exhausted, allocate TSG 0xcef8000b of client 0xc1d00028.
  3. Allocate the same handle again.
  4. Allocate three TSGs of client 0xc1d0221f.
  5. On a fresh GPU, allocate a TSG whose PROMOTE_FAULT_METHOD_BUFFERS control fails.
  6. Allocate a TSG whose alloc RPC itself fails (duplicate handle). This must not free GSP-RM's existing object.

Command: ./verify.sh /Volumes/SammyDisk/oss-contrib-work/nvidia__open-gpu-kernel-modules@1379 61dcc937 fix/issue-1379
(builds and runs the harness on git archive of 615.71.09 and of the patched commit)

Result (excerpt of the output; each run has 15 checks):

===== 61dcc937 (61dcc937): 615.71.09
== 2. Vidmem exhausted: client A allocates TSG 0xcef8000b
NVRM: GPU0 nvCheckOkFailedNoLog: Check failed: Out of memory [NV_ERR_NO_MEMORY] (0x00000051) returned from ctxBufPoolReserve(pGpu, pKernelChannelGroup->pCtxBufPool, bufInfoList, bufCount) @ kernel_channel_group_api.c:590
  PASS: the alloc fails with NV_ERR_NO_MEMORY
  FAIL: GSP-RM no longer has TSG 0xcef8000b of the failed alloc
  FAIL: kernel-RM and GSP-RM TSG IDs match
== 3. Vidmem available again: client A retries TSG 0xcef8000b
NVRM: GPU0 rpcRmApiAlloc_GSP: GspRmAlloc failed: hClient=0xc1d00028; hParent=0xbeef0003; hObject=0xcef8000b; hClass=0x0000a06c; paramsSize=0x0000001c; paramsStatus=0x00000019; status=0x00000019
  FAIL: the retry with the same handle succeeds
== 4. Client B allocates three TSGs (a new GL/Vulkan context)
NVRM: GPU0 rpcRmApiAlloc_GSP: GspRmAlloc failed: hClient=0xc1d0221f; hParent=0xbeef0003; hObject=0xcef80008; hClass=0x0000a06c; paramsSize=0x0000001c; paramsStatus=0x00000063; status=0x00000063
  FAIL: client B's TSG alloc succeeds
  ...
FAILED: 9 check(s) failed
exit status: 1

===== fix/issue-1379 (22c1638a): nvidia: free the GSP TSG when channel group construction fails after the alloc RPC
  ...
PASSED: 0 check(s) failed
exit status: 0
  • 615.71.09: 6 of the 15 checks passed and 9 failed. The two GspRmAlloc failures it prints are identical to the lines in the
    bug report.
  • With the patch: all 15 checks passed.
  • Neither run reached an abort stub or produced an ASan report.

Compile check: I compiled the patched file with the exact command from
make -n -C src/nvidia TARGET_ARCH=x86_64 CC=clang, plus clang --target=x86_64-linux-gnu -Werror. It returned rc=0 with no
warnings, and the unmodified file gives the same result. GCC was not available locally.

…the alloc RPC

kchangrpapiConstruct_IMPL() sends the TSG alloc RPC and can still fail
afterwards: ctxBufPoolReserve() running out of vidmem, the
PROMOTE_FAULT_METHOD_BUFFERS control, or listAppendValue(). A failed
constructor is never destructed, so the RPC free that bRpcFree asks for
never happens and GSP-RM keeps the TSG after kernel-RM has released its
handle and grpID.

Since 615.71.09 kernel-RM passes its grpID to GSP-RM, so every later TSG
or bare channel allocation that gets the leaked grpID fails on GSP-RM
with NV_ERR_STATE_IN_USE, and the client reusing the handle fails with
NV_ERR_INSERT_DUPLICATE_NAME, until the leaking client exits.

Free the object on GSP/host in the failure path once the alloc RPC has
succeeded, as kchannelConstruct_IMPL() already does for channels.
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wayland EGL crash (libnvidia-eglcore SEGV_MAPERR 0xc) and VKD3D vkCreateDevice failure (vr -3)

2 participants