On Wed, Dec 20, 2023 at 05:17:49PM +0100, Thomas Hellström wrote:
On 12/20/23 17:13, Lucas De Marchi wrote:
On Wed, Dec 20, 2023 at 03:42:14PM +0100, Thomas Hellström wrote:
If using the VM_BIND_OP_UNMAP_ALL without any bound vmas for the
vm, we will end up dereferencin an uninitialized variable and leak a
dereferencing
bo lock. Fix this.
Reported-by: Dafna Hirschfeld <dhirschfeld@xxxxxxxxx>
Closes: https://lore.kernel.org/intel-xe/jrwua7ckbiozfcaodx4gg2h4taiuxs53j5zlpf3qzvyhyiyl2d@pbs3plurokrj/
Suggested-by: Dafna Hirschfeld <dhirschfeld@xxxxxxxxx>
Fixes: 9f232f4ae249 ("drm/xe: Port Xe to GPUVA")
^ this newline needs to be removed so `git log --format="%(trailers)"'
shows everything, not only your s-o-b.
Will fix these.
Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/xe/xe_vm.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 1ca917b8315c..127842656a23 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -2063,9 +2063,11 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm,
struct xe_bo *bo,
if (err)
return ERR_PTR(err);
- vm_bo = drm_gpuvm_bo_find(&vm->gpuvm, obj);
- if (!vm_bo)
- break;
+ vm_bo = drm_gpuvm_bo_obtain(&vm->gpuvm, obj);
if the issue with that we don't have any bound vmas, why are we going to
create a new one just to be released?
The expected outcome of this operation, AFAICT is, rather than to
error, to create an ops with an empty list of operations, so we could
in theory kmalloc ops and just initialize its list. However that would
be fragile and second-guessing what gpuvm does internally, so I opted
for this solution instead.
ok,
Acked-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Adding dri-devel and Danilo to Cc as it looks like we are the first
users of this api.
The NULL vs ptr-error between _find and _obtain looked suspicious when I
first looked, but they match the current implementation.
thanks
Lucas De Marchi
/Thomas
Lucas De Marchi
+ if (IS_ERR(vm_bo)) {
+ xe_bo_unlock(bo);
+ return ERR_CAST(vm_bo);
+ }
ops = drm_gpuvm_bo_unmap_ops_create(vm_bo);
drm_gpuvm_bo_put(vm_bo);
--
2.42.0
