Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] NULL pointer dereference when printing imported BOs in debugfs. -- commit be759dc89504ad9041bef159ec02b120e04cf61b Author: Thadeu Lima de Souza Cascardo <[email protected]> Revert "drm/amdgpu: debugfs: avoid extra EOLs in amdgpu_gem_info" This commit reverts an earlier change that removed newlines when no fences were attached to a dma_resv struct. The original change caused multiple BOs to be printed on the same line, reducing debugfs readability. > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index 5d9d137209b6f..1b6c32a177fb6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -1701,9 +1701,8 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, > struct seq_file *m) [Severity: High] This is a pre-existing issue, but can the access to dma_buf->file cause a NULL pointer dereference? In amdgpu_bo_print_info(), earlier in this function: dma_buf = READ_ONCE(bo->tbo.base.dma_buf); attachment = READ_ONCE(bo->tbo.base.import_attach); if (attachment) seq_printf(m, " imported from ino:%llu", file_inode(dma_buf->file)->i_ino); If attachment is non-NULL, dma_buf is dereferenced via file_inode(dma_buf->file)->i_ino. However, bo->tbo.base.dma_buf can be NULL for imported BOs in several scenarios: - BOs imported via amdgpu_gem_prime_import() allocate a fresh GEM object via amdgpu_dma_buf_create_obj(), which does not populate obj->dma_buf and relies entirely on obj->import_attach. - When userspace closes the GEM handle of a BO, drm_gem_object_handle_put_unlocked() calls drm_gem_object_exported_dma_buf_free(), explicitly setting obj->dma_buf = NULL, even though the BO itself may still be kept alive if it is mapped. When debugfs iterates over these BOs, reading dma_buf->file dereferences a NULL pointer. Since this can be triggered by reading debugfs files like amdgpu_vm_info, could this deterministically oops the system? > if (dma_resv_trylock(bo->tbo.base.resv)) { > dma_resv_describe(bo->tbo.base.resv, m); > dma_resv_unlock(bo->tbo.base.resv); > - } else { > - seq_puts(m, "\n"); > } > + seq_puts(m, "\n"); > > return size; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
