Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Medium] Memory and reference leak of vbuf, vbuf->objs, and vbuf->data_buf on error paths during command queuing. -- commit 170d71262d2472089fb4a6a1649487aa23d08e9a Author: Jason Macnak <[email protected]> drm/virtio: Don't detach GEM from a non-created context Applies the same treatment as commit 7cf6dd467e87 ("drm/virtio: Don't attach GEM to a non-created context in gem_object_open()") to virtio_gpu_gem_object_close() to avoid trying to detach a resource that was never attached due to a context never being created when context_init is supported. > diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c > b/drivers/gpu/drm/virtio/virtgpu_gem.c > index 435d37d360340..66c3f6f74e9c6 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_gem.c > +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c > @@ -139,13 +139,15 @@ void virtio_gpu_gem_object_close(struct drm_gem_object > *obj, > if (!vgdev->has_virgl_3d) > return; [ ... ] > + if (vfpriv->context_created) { > + objs = virtio_gpu_array_alloc(1); > + if (!objs) > + return; > + virtio_gpu_array_add_obj(objs, obj); > > - virtio_gpu_cmd_context_detach_resource(vgdev, vfpriv->ctx_id, > - objs); > + virtio_gpu_cmd_context_detach_resource(vgdev, vfpriv->ctx_id, > + objs); [Severity: Medium] This is a pre-existing issue, but does this code leak the objs array and GEM object references on error paths deeper in the command queuing stack? When virtio_gpu_cmd_context_detach_resource() queues the command, it calls into virtio_gpu_queue_ctrl_buffer(). If an error occurs further down, the vbuf and vbuf->objs may not be cleaned up. For example, in drivers/gpu/drm/virtio/virtgpu_vq.c:virtio_gpu_queue_fenced_ctrl_buffer(): if (!sgt) { if (fence && vbuf->objs) virtio_gpu_array_unlock_resv(vbuf->objs); return -ENOMEM; } It returns -ENOMEM without calling free_vbuf() or releasing vbuf->objs. Similarly, in drivers/gpu/drm/virtio/virtgpu_vq.c:virtio_gpu_queue_ctrl_sgs(): if (!drm_dev_enter(vgdev->ddev, &idx)) { if (fence && vbuf->objs) virtio_gpu_array_unlock_resv(vbuf->objs); free_vbuf(vgdev, vbuf); return -ENODEV; } It frees the vbuf but fails to release vbuf->objs and its GEM references (unlike the success path in virtio_gpu_dequeue_ctrl_func). Since callers like virtio_gpu_cmd_context_detach_resource() ignore the return value, is there a mechanism to prevent these resources from leaking when command queuing fails? > + } > virtio_gpu_notify(vgdev); > } > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
