On 08/12/2023 10:46, Thierry Reding wrote:
From: Thierry Reding
Commit 12c9b05da918 ("drm/nouveau/imem: support allocations not
preserved across suspend") uses container_of() to cast from struct
nvkm_memory to struct nvkm_instobj, assuming that all instance objects
are derived from struct nvkm_instobj. For the gk20a family that's not
the case and they are derived from struct nvkm_memory instead. This
causes some subtle data corruption (nvkm_instobj.preserve ends up
mapping to gk20a_instobj.vaddr) that causes a NULL pointer dereference
in gk20a_instobj_acquire_iommu() (and possibly elsewhere) and also
prevents suspend/resume from working.
Fix this by making struct gk20a_instobj derive from struct nvkm_instobj
instead.
Fixes: 12c9b05da918 ("drm/nouveau/imem: support allocations not preserved across
suspend")
Reported-by: Jonathan Hunter
Signed-off-by: Thierry Reding
---
Note that this was probably subtly wrong before the above-mentioned
commit already, but I don't think we've seen any reports that would
indicate any actual failures related to this before. So I think it's
good enough to apply this fix for v6.7. The next closest thing would
be commit d8e83994aaf6 ("drm/nouveau/imem: improve management of
instance memory"), but that's 8 years old (Linux v4.3)...
---
.../drm/nouveau/nvkm/subdev/instmem/gk20a.c| 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
index 1b811d6972a1..201022ae9214 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c
@@ -49,14 +49,14 @@
#include
struct gk20a_instobj {
- struct nvkm_memory memory;
+ struct nvkm_instobj base;
struct nvkm_mm_node *mn;
struct gk20a_instmem *imem;
/* CPU mapping */
u32 *vaddr;
};
-#define gk20a_instobj(p) container_of((p), struct gk20a_instobj, memory)
+#define gk20a_instobj(p) container_of((p), struct gk20a_instobj, base.memory)
/*
* Used for objects allocated using the DMA API
@@ -148,7 +148,7 @@ gk20a_instobj_iommu_recycle_vaddr(struct
gk20a_instobj_iommu *obj)
list_del(>vaddr_node);
vunmap(obj->base.vaddr);
obj->base.vaddr = NULL;
- imem->vaddr_use -= nvkm_memory_size(>base.memory);
+ imem->vaddr_use -= nvkm_memory_size(>base.base.memory);
nvkm_debug(>base.subdev, "vaddr used: %x/%x\n", imem->vaddr_use,
imem->vaddr_max);
}
@@ -283,7 +283,7 @@ gk20a_instobj_map(struct nvkm_memory *memory, u64 offset,
struct nvkm_vmm *vmm,
{
struct gk20a_instobj *node = gk20a_instobj(memory);
struct nvkm_vmm_map map = {
- .memory = >memory,
+ .memory = >base.memory,
.offset = offset,
.mem = node->mn,
};
@@ -391,8 +391,8 @@ gk20a_instobj_ctor_dma(struct gk20a_instmem *imem, u32
npages, u32 align,
return -ENOMEM;
*_node = >base;
- nvkm_memory_ctor(_instobj_func_dma, >base.memory);
- node->base.memory.ptrs = _instobj_ptrs;
+ nvkm_memory_ctor(_instobj_func_dma, >base.base.memory);
+ node->base.base.memory.ptrs = _instobj_ptrs;
node->base.vaddr = dma_alloc_attrs(dev, npages << PAGE_SHIFT,
>handle, GFP_KERNEL,
@@ -438,8 +438,8 @@ gk20a_instobj_ctor_iommu(struct gk20a_instmem *imem, u32
npages, u32 align,
*_node = >base;
node->dma_addrs = (void *)(node->pages + npages);
- nvkm_memory_ctor(_instobj_func_iommu, >base.memory);
- node->base.memory.ptrs = _instobj_ptrs;
+ nvkm_memory_ctor(_instobj_func_iommu, >base.base.memory);
+ node->base.base.memory.ptrs = _instobj_ptrs;
/* Allocate backing memory */
for (i = 0; i < npages; i++) {
@@ -533,7 +533,7 @@ gk20a_instobj_new(struct nvkm_instmem *base, u32 size, u32
align, bool zero,
else
ret = gk20a_instobj_ctor_dma(imem, size >> PAGE_SHIFT,
align, );
- *pmemory = node ? >memory : NULL;
+ *pmemory = node ? >base.memory : NULL;
if (ret)
return ret;
Tested-by: Jon Hunter
Thanks!
Jon
--
nvpublic
