Skip to content

Commit 9af1b6e

Browse files
deepanshu406digetx
authored andcommitted
drm/virtio: use uninterruptible resv lock for plane updates
virtio_gpu_cursor_plane_update() and virtio_gpu_resource_flush() lock the framebuffer BO's dma_resv via virtio_gpu_array_lock_resv() and ignore its return value. The function can fail with -EINTR from dma_resv_lock_interruptible() (signal during lock wait) or with -ENOMEM from dma_resv_reserve_fences() (fence slot allocation), leaving the resv lock not held. The queue path then walks the object array and calls dma_resv_add_fence(), which requires the lock held; with lockdep enabled this trips dma_resv_assert_held(): WARNING: drivers/dma-buf/dma-resv.c:296 at dma_resv_add_fence+0x71e/0x840 Call Trace: virtio_gpu_array_add_fence virtio_gpu_queue_ctrl_sgs virtio_gpu_queue_fenced_ctrl_buffer virtio_gpu_cursor_plane_update drm_atomic_helper_commit_planes drm_atomic_helper_commit_tail commit_tail drm_atomic_helper_commit drm_atomic_commit drm_atomic_helper_update_plane __setplane_atomic drm_mode_cursor_universal drm_mode_cursor_common drm_mode_cursor_ioctl drm_ioctl __x64_sys_ioctl Beyond the WARN, mutating the dma_resv fence list without the lock races with concurrent readers/writers and can corrupt the list. Both call sites run inside the .atomic_update plane callback, which DRM atomic helpers do not allow to fail (by the time it runs, the commit has been signed off to userspace and there is no clean rollback path). Moving the lock acquisition to .prepare_fb was rejected because the broader lock scope deadlocks against other BO locking paths in the same atomic commit. Introduce virtio_gpu_lock_one_resv_uninterruptible() that uses dma_resv_lock() instead of dma_resv_lock_interruptible(). This eliminates the -EINTR failure mode -- the realistic syzbot trigger -- without extending the lock hold across the commit. The helper locks a single BO and rejects nents > 1 with -EINVAL; both fix sites lock exactly one BO. Use it from virtio_gpu_cursor_plane_update() and virtio_gpu_resource_flush(); check the return value to handle the remaining -ENOMEM case from dma_resv_reserve_fences() by freeing the objs and skipping the plane update for that frame. The framebuffer BOs touched here are not shared with other contexts and lock contention is expected to be brief, so the loss of signal-interruptibility is acceptable. Other callers of virtio_gpu_array_lock_resv() (the ioctl paths) continue to use the interruptible variant. The bug was reported by syzbot, triggered via fault injection (fail_nth) on the DRM_IOCTL_MODE_CURSOR path, which forces the -ENOMEM branch in dma_resv_reserve_fences(). Reported-by: syzbot+72bd3dd3a5d5f39a0271@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=72bd3dd3a5d5f39a0271 Fixes: 5cfd31c ("drm/virtio: fix virtio_gpu_cursor_plane_update().") Cc: stable@vger.kernel.org Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> Link: https://patch.msgid.link/20260519082247.34470-1-kartikey406@gmail.com
1 parent 457b046 commit 9af1b6e

3 files changed

Lines changed: 26 additions & 2 deletions

File tree

drivers/gpu/drm/virtio/virtgpu_drv.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ virtio_gpu_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents
317317
void virtio_gpu_array_add_obj(struct virtio_gpu_object_array *objs,
318318
struct drm_gem_object *obj);
319319
int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs);
320+
int virtio_gpu_lock_one_resv_uninterruptible(struct virtio_gpu_object_array *objs);
320321
void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs);
321322
void virtio_gpu_array_add_fence(struct virtio_gpu_object_array *objs,
322323
struct dma_fence *fence);

drivers/gpu/drm/virtio/virtgpu_gem.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,23 @@ int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs)
238238
return ret;
239239
}
240240

241+
int virtio_gpu_lock_one_resv_uninterruptible(struct virtio_gpu_object_array *objs)
242+
{
243+
int ret;
244+
245+
if (objs->nents != 1)
246+
return -EINVAL;
247+
248+
dma_resv_lock(objs->objs[0]->resv, NULL);
249+
250+
ret = dma_resv_reserve_fences(objs->objs[0]->resv, 1);
251+
if (ret) {
252+
virtio_gpu_array_unlock_resv(objs);
253+
return ret;
254+
}
255+
return 0;
256+
}
257+
241258
void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs)
242259
{
243260
if (objs->nents == 1) {

drivers/gpu/drm/virtio/virtgpu_plane.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,10 @@ static void virtio_gpu_resource_flush(struct drm_plane *plane,
215215
if (!objs)
216216
return;
217217
virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
218-
virtio_gpu_array_lock_resv(objs);
218+
if (virtio_gpu_lock_one_resv_uninterruptible(objs)) {
219+
virtio_gpu_array_put_free(objs);
220+
return;
221+
}
219222
virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y,
220223
width, height, objs,
221224
vgplane_st->fence);
@@ -459,7 +462,10 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane,
459462
if (!objs)
460463
return;
461464
virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
462-
virtio_gpu_array_lock_resv(objs);
465+
if (virtio_gpu_lock_one_resv_uninterruptible(objs)) {
466+
virtio_gpu_array_put_free(objs);
467+
return;
468+
}
463469
virtio_gpu_cmd_transfer_to_host_2d
464470
(vgdev, 0,
465471
plane->state->crtc_w,

0 commit comments

Comments
 (0)