Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The check for `adev->mman.buffer_funcs_enabled` is vulnerable to a Time-Of-Check to Time-Of-Use (TOCTOU) race with concurrent runtime PM suspend, potentially leading to a PCIe Master Abort system crash or scheduler corruption. --
commit 7e4c0d8b57d6cf1fda7d356466ca2334810cad8a Author: Mike Lothian <[email protected]> drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended amdgpu_dma_buf_map() adds VRAM to the allowed domains for a peer2peer attachment, so ttm_bo_validate() can migrate the buffer from GTT into VRAM. While the exporting device is runtime suspended its SDMA rings are down and the move fails. This patch only requests VRAM when the device can perform the move. > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > index b33c300e26e2e..d1dae5f3fee5e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > @@ -191,7 +191,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct > dma_buf_attachment *attach, > unsigned int domains = AMDGPU_GEM_DOMAIN_GTT; > > if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM && > - attach->peer2peer) { > + attach->peer2peer && > + adev->mman.buffer_funcs_enabled) { [Severity: High] In drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c at line 194 in amdgpu_dma_buf_map(), does this lockless check introduce a time-of-check to time-of-use race with the runtime PM suspend path? When userspace (e.g., a compositor) issues buffer validations on an exported dGPU buffer in a multi-GPU environment, the exporter's BO dma_resv lock is held. However, if the BO is in GTT, amdgpu_device_suspend() does not block on this lock since it only evicts VRAM. This means amdgpu_device_suspend() can proceed concurrently, call amdgpu_ttm_disable_buffer_funcs() to set buffer_funcs_enabled = false, destroy the scheduler entities, and transition the device to D3cold. When the subsequent ttm_bo_validate() reaches amdgpu_bo_move(), could it see buffer_funcs_enabled as false and fall back to ttm_bo_move_memcpy(), causing a PCIe Master Abort crash on a powered-down device? Alternatively, could it see buffer_funcs_enabled as true but submit a job to an already destroyed drm_sched_entity? > bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; > domains |= AMDGPU_GEM_DOMAIN_VRAM; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
