Re: [PATCH v6] drm/virtio: Add option to disable KMS support
On 3/3/23 02:35, Rob Clark wrote: > @@ -223,12 +223,15 @@ int virtio_gpu_init(struct virtio_device *vdev, struct > drm_device *dev) > num_scanouts, _scanouts); > vgdev->num_scanouts = min_t(uint32_t, num_scanouts, > VIRTIO_GPU_MAX_SCANOUTS); > - if (!vgdev->num_scanouts) { > - DRM_ERROR("num_scanouts is zero\n"); > - ret = -EINVAL; > - goto err_scanouts; > + > + if (IS_ENABLED(CONFIG_DRM_VIRTIO_GPU_KMS) || !vgdev->num_scanouts) { Unfortunately the IS_ENABLED check needs to be inverted here. Secondly, with the IS_ENABLED check fixed and CONFIG_DRM_VIRTIO_GPU_KMS disabled, kernel crashes with a NULL deref on boot after getting the VIRTIO_GPU_EVENT_DISPLAY from host. == [0.870144] BUG: KASAN: null-ptr-deref in drm_kms_helper_hotplug_event+0x2b/0x50 [0.870588] Read of size 8 at addr 0010 by task kworker/0:1/14 [0.870588] [0.870588] CPU: 0 PID: 14 Comm: kworker/0:1 Not tainted 6.2.0-next-20230303+ #387 [0.871441] scsi host0: ahci [0.870588] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014 [0.870588] Workqueue: events virtio_gpu_dequeue_ctrl_func [0.870588] Call Trace: [0.870588] [0.872841] scsi host1: ahci [0.870588] dump_stack_lvl+0x46/0x70 [0.870588] kasan_report+0xbb/0xf0 [0.870588] ? drm_kms_helper_hotplug_event+0x2b/0x50 [0.870588] drm_kms_helper_hotplug_event+0x2b/0x50 [0.870588] virtio_gpu_dequeue_ctrl_func+0x143/0x500 [0.870588] ? lock_is_held_type+0xd8/0x130 [0.870588] ? virtio_gpu_free_vbufs+0x40/0x40 [0.875105] scsi host2: ahci [0.870588] process_one_work+0x4f5/0x9a0 [0.870588] ? pwq_dec_nr_in_flight+0x100/0x100 [0.870588] ? spin_bug+0xe0/0xe0 [0.870588] worker_thread+0x8c/0x610 [0.870588] ? process_one_work+0x9a0/0x9a0 [0.870588] kthread+0x15a/0x190 [0.870588] ? kthread_complete_and_exit+0x20/0x20 [0.870588] ret_from_fork+0x1f/0x30 [0.877201] scsi host3: ahci [0.870588] [0.870588] == I'll send a follow up fix. -- Best regards, Dmitry
Re: [PATCH v6] drm/virtio: Add option to disable KMS support
On 3/3/23 02:35, Rob Clark wrote: > From: Rob Clark > > Add a build option to disable modesetting support. This is useful in > cases where the guest only needs to use the GPU in a headless mode, or > (such as in the CrOS usage) window surfaces are proxied to a host > compositor. > > As the modesetting ioctls are a big surface area for potential security > bugs to be found (it's happened in the past, we should assume it will > again in the future), it makes sense to have a build option to disable > those ioctls in cases where they serve no legitimate purpose. > > v2: Use more if (IS_ENABLED(...)) > v3: Also permit the host to advertise no scanouts > v4: Spiff out commit msg > v5: Make num_scanouts==0 and DRM_VIRTIO_GPU_KMS=n behave the same > v6: Drop conditionally building virtgpu_display.c and early-out of > it's init/fini fxns instead > > Signed-off-by: Rob Clark > Reviewed-by: Dmitry Osipenko > Reviewed-by: Javier Martinez Canillas > --- Applied to misc-next -- Best regards, Dmitry
Re: [PATCH v6] drm/virtio: Add option to disable KMS support
On Thu, Mar 02, 2023 at 03:35:06PM -0800, Rob Clark wrote: > From: Rob Clark > > Add a build option to disable modesetting support. This is useful in > cases where the guest only needs to use the GPU in a headless mode, or > (such as in the CrOS usage) window surfaces are proxied to a host > compositor. > > As the modesetting ioctls are a big surface area for potential security > bugs to be found (it's happened in the past, we should assume it will > again in the future), it makes sense to have a build option to disable > those ioctls in cases where they serve no legitimate purpose. > > v2: Use more if (IS_ENABLED(...)) > v3: Also permit the host to advertise no scanouts > v4: Spiff out commit msg > v5: Make num_scanouts==0 and DRM_VIRTIO_GPU_KMS=n behave the same > v6: Drop conditionally building virtgpu_display.c and early-out of > it's init/fini fxns instead Reviewed-by: Gerd Hoffmann