Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] Erroneous release of file descriptor 0 on early error returns due to 0-initialized out_fence_fd. - [High] Leak of DRM event memory and event space quota on early submission failure. - [High] Premature reset of in_syncobjs on error paths breaks ioctl restart and destroys dependencies. -- commit 1f57c5e182bb2310a6b5538ff7f993fd7e45dee5 Author: Peiyang He <[email protected]> drm/virtio: fix NULL pointer dereference on fence allocation failure This commit adds a missing NULL check after allocating a fence in virtio_gpu_init_submit() to prevent a NULL pointer dereference under memory pressure. > diff --git a/drivers/gpu/drm/virtio/virtgpu_submit.c > b/drivers/gpu/drm/virtio/virtgpu_submit.c > --- a/drivers/gpu/drm/virtio/virtgpu_submit.c > +++ b/drivers/gpu/drm/virtio/virtgpu_submit.c > @@ -389,10 +389,13 @@ static int virtio_gpu_init_submit(struct > virtio_gpu_submit *submit, > if ((exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_OUT) || > exbuf->num_out_syncobjs || > exbuf->num_bo_handles || > - drm_fence_event) > + drm_fence_event) { > out_fence = virtio_gpu_fence_alloc(vgdev, fence_ctx, ring_idx); > - else > + if (!out_fence) > + return -ENOMEM; [Severity: High] This isn't a bug introduced by this patch, but returning early here exposes a pre-existing issue with file descriptor handling. Because virtio_gpu_init_submit() initializes the submit struct using memset(submit, 0, sizeof(*submit)), submit->out_fence_fd is left as 0. If we return -ENOMEM here, the caller eventually invokes virtio_gpu_cleanup_submit(), which checks: if (submit->out_fence_fd >= 0) put_unused_fd(submit->out_fence_fd); Since 0 is >= 0, this will erroneously release file descriptor 0 (typically stdin) back to the kernel, potentially allowing subsequent open() calls to hijack the file descriptor. Could out_fence_fd be initialized to -1 before taking this early return? [Severity: High] This is a pre-existing issue, but does taking an error path (such as this new -ENOMEM return) break system call restarts? When the submission ioctl fails, the error path calls virtio_gpu_cleanup_submit(). This function unconditionally calls virtio_gpu_reset_syncobjs(), replacing all input syncobj fences with NULL. When the kernel automatically restarts the ioctl (e.g. after -ERESTARTSYS is returned during a signal interruption), the dependencies are already destroyed, causing drm_syncobj_find_fence() to return -EINVAL and breaking userspace synchronization state. > + } else { > out_fence = NULL; > + } > > if (drm_fence_event) { > err = virtio_gpu_fence_event_create(dev, file, out_fence, > ring_idx); [Severity: High] This is a pre-existing issue, but how does the code handle event cleanup if an error occurs later in the submission process? If virtio_gpu_fence_event_create() succeeds, it allocates the event and deducts from file_priv->event_space. If a subsequent initialization step fails, virtio_gpu_cleanup_submit() drops the fence reference via dma_fence_put(&submit->out_fence->f). However, because virtio_gpu_fence_ops lacks a .release callback, dma_fence_release() simply frees the fence without ever calling drm_event_cancel_free(), leaking both the event memory and the client's event space quota. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/00efe4ba92889b14+20260909091114.2622550-1-peiyang...@smail.nju.edu.cn?part=1
