Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/imagination: missing cleanup of deferred GPUVM BOs in pvr_vm_map() --
commit 7b2c8feb3b00c98681f3c0f485d9d8ac9c58c5eb Author: Gyeyoung Baek <[email protected]> drm/imagination: Stop locking reservations around map and unmap The GPUVA lists are now protected by gpuva.lock instead of the reservations, so map and unmap no longer need to lock the reservations. Nothing else on that path relies on them. > diff --git a/drivers/gpu/drm/imagination/pvr_vm.c > b/drivers/gpu/drm/imagination/pvr_vm.c > index 3921b765688e1..2f1bbcfdb60e3 100644 > --- a/drivers/gpu/drm/imagination/pvr_vm.c > +++ b/drivers/gpu/drm/imagination/pvr_vm.c [ ... ] > @@ -739,15 +729,6 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct > pvr_gem_object *pvr_obj, > u64 pvr_obj_offset, u64 device_addr, u64 size) > { > struct pvr_vm_bind_op bind_op = {0}; > - struct drm_gpuvm_exec vm_exec = { > - .vm = &vm_ctx->gpuvm_mgr, > - .flags = DRM_EXEC_INTERRUPTIBLE_WAIT | > - DRM_EXEC_IGNORE_DUPLICATES, > - .extra = { > - .fn = pvr_vm_lock_extra, > - .priv = &bind_op, > - }, > - }; > > int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj, > pvr_obj_offset, device_addr, [Severity: Medium] If pvr_vm_bind_op_map_init() fails after creating a gpuvm_bo, does this code leak the deferred BO? Looking at pvr_vm_bind_op_map_init(), if it fails (e.g., due to ENOMEM or an import mapping failure in pvr_gem_object_get_pages_sgt()), the error path calls pvr_vm_bind_op_fini() which defers the BO cleanup: pvr_vm_bind_op_fini() { drm_gpuvm_bo_put_deferred(bind_op->gpuvm_bo); ... } However, the early return path in pvr_vm_map() (right after the call to pvr_vm_bind_op_map_init()) skips calling drm_gpuvm_bo_deferred_cleanup(): int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj, pvr_obj_offset, device_addr, size); if (err) return err; Since pvr_vm_map() is reachable via the PVR_IOCTL_VM_MAP ioctl, could a faulty or malicious userspace trigger this error path repeatedly and cause the deferred list to grow unbounded, leaking kernel memory until the VM context is destroyed? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
