virtio_gpu_alloc_vbufs() creates a kmem_cache with the fixed name "virtio-gpu-vbufs" for every device and destroys it from virtio_gpu_release(), which only runs once the last drm_device reference is dropped. If a reference outlives the PCI removal (for instance an open /dev/fbN), a rescan probes the device again while the old cache still exists:
kmem_cache of name 'virtio-gpu-vbufs' already exists WARNING: mm/slab_common.c:111 at __kmem_cache_create_args Call Trace: virtio_gpu_alloc_vbufs virtio_gpu_init virtio_gpu_probe ... pci_rescan_bus rescan_store kmem_cache_create() then fails and the new device does not probe. A vbuf is 216 bytes and lands in kmalloc-256 anyway, so the dedicated cache buys nothing. Use kzalloc()/kfree() and drop the per-device cache. Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=1b129b44597a126d2d79 Fixes: dc5698e80cf7 ("Add virtio gpu driver.") Signed-off-by: Nguyen Ngoc Thang <[email protected]> --- v2: (Hillf Danton) drop the dedicated cache and use kzalloc()/kfree() instead of sharing one cache across devices. v1: https://lore.kernel.org/r/[email protected] drivers/gpu/drm/virtio/virtgpu_drv.h | 3 --- drivers/gpu/drm/virtio/virtgpu_kms.c | 8 -------- drivers/gpu/drm/virtio/virtgpu_vq.c | 23 +++-------------------- 3 files changed, 3 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 9df4c7117341..2639d9ec0470 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -261,7 +261,6 @@ struct virtio_gpu_device { struct virtio_gpu_queue ctrlq; struct virtio_gpu_queue cursorq; bool vqs_released; - struct kmem_cache *vbufs; atomic_t pending_commands; @@ -361,8 +360,6 @@ void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev, void virtio_gpu_array_put_free_work(struct work_struct *work); /* virtgpu_vq.c */ -int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev); -void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev); void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev); void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev, struct virtio_gpu_object *bo, diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 1d4d3bf46a20..047b591b5da2 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -264,11 +264,6 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) DRM_ERROR("failed to find virt queues\n"); goto err_vqs; } - ret = virtio_gpu_alloc_vbufs(vgdev); - if (ret) { - DRM_ERROR("failed to alloc vbufs\n"); - goto err_vbufs; - } /* get display info */ virtio_cread_le(vgdev->vdev, struct virtio_gpu_config, @@ -324,8 +319,6 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) virtio_reset_device(vgdev->vdev); virtio_gpu_modeset_fini(vgdev); err_scanouts: - virtio_gpu_free_vbufs(vgdev); -err_vbufs: vgdev->vdev->config->del_vqs(vgdev->vdev); err_vqs: dev->dev_private = NULL; @@ -365,7 +358,6 @@ void virtio_gpu_release(struct drm_device *dev) return; virtio_gpu_modeset_fini(vgdev); - virtio_gpu_free_vbufs(vgdev); virtio_gpu_cleanup_cap_cache(vgdev); if (vgdev->has_host_visible) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index c02c03c10d92..5eb78d69f561 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -70,30 +70,13 @@ void virtio_gpu_cursor_ack(struct virtqueue *vq) schedule_work(&vgdev->cursorq.dequeue_work); } -int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev) -{ - vgdev->vbufs = kmem_cache_create("virtio-gpu-vbufs", - VBUFFER_SIZE, - __alignof__(struct virtio_gpu_vbuffer), - 0, NULL); - if (!vgdev->vbufs) - return -ENOMEM; - return 0; -} - -void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev) -{ - kmem_cache_destroy(vgdev->vbufs); - vgdev->vbufs = NULL; -} - /* For drm_panic */ static struct virtio_gpu_vbuffer* virtio_gpu_panic_get_vbuf(struct virtio_gpu_device *vgdev, int size) { struct virtio_gpu_vbuffer *vbuf; - vbuf = kmem_cache_zalloc(vgdev->vbufs, GFP_ATOMIC); + vbuf = kzalloc(VBUFFER_SIZE, GFP_ATOMIC); vbuf->buf = (void *)vbuf + sizeof(*vbuf); vbuf->size = size; @@ -110,7 +93,7 @@ virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev, { struct virtio_gpu_vbuffer *vbuf; - vbuf = kmem_cache_zalloc(vgdev->vbufs, GFP_KERNEL | __GFP_NOFAIL); + vbuf = kzalloc(VBUFFER_SIZE, GFP_KERNEL | __GFP_NOFAIL); BUG_ON(size > MAX_INLINE_CMD_SIZE || size < sizeof(struct virtio_gpu_ctrl_hdr)); @@ -205,7 +188,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev, if (vbuf->resp_size > MAX_INLINE_RESP_SIZE) kfree(vbuf->resp_buf); kvfree(vbuf->data_buf); - kmem_cache_free(vgdev->vbufs, vbuf); + kfree(vbuf); } void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev) -- 2.43.0
