2021-05-17 08:53:26

by Yongji Xie

Subject: [PATCH 1/3] drm/virtio: Fixes a potential NULL pointer dereference on probe failure

The dev->dev_private might not be allocated if virtio_gpu_pci_quirk()
or virtio_gpu_init() failed. In this case, we should avoid the cleanup
in virtio_gpu_release().

Signed-off-by: Xie Yongji <[email protected]>
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index b375394193be..aa532ad31a23 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -264,6 +264,9 @@ void virtio_gpu_release(struct drm_device *dev)
{
struct virtio_gpu_device *vgdev = dev->dev_private;

+ if (!vgdev)
+ return;
+
virtio_gpu_modeset_fini(vgdev);
virtio_gpu_free_vbufs(vgdev);
virtio_gpu_cleanup_cap_cache(vgdev);
--
2.11.0

2021-05-17 13:16:09

by Yongji Xie

Subject: [PATCH 3/3] drm/virtio: free virtqueues on probe failure

We should call virtio_gpu_deinit() to free virtqueues when
drm_dev_register() failed.

Signed-off-by: Xie Yongji <[email protected]>
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index a21dc3ad6f88..31ba4f344ab9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -121,11 +121,13 @@ static int virtio_gpu_probe(struct virtio_device *vdev)

ret = drm_dev_register(dev, 0);
if (ret)
- goto err_free;
+ goto err_deinit;

drm_fbdev_generic_setup(vdev->priv, 32);
return 0;

+err_deinit:
+ virtio_gpu_deinit(dev);
err_free:
drm_dev_put(dev);
return ret;
--
2.11.0

2021-05-17 13:51:41

by Yongji Xie

Subject: [PATCH 2/3] drm/virtio: Fix double free on probe failure

The virtio_gpu_init() will free vgdev and vgdev->vbufs on failure.
But such failure will be caught by virtio_gpu_probe() and then
virtio_gpu_release() will be called to do some cleanup which
will free vgdev and vgdev->vbufs again. So let's set dev->dev_private
to NULL to avoid double free.

Signed-off-by: Xie Yongji <[email protected]>
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index aa532ad31a23..f3379059f324 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -234,6 +234,7 @@ int virtio_gpu_init(struct drm_device *dev)
err_vbufs:
vgdev->vdev->config->del_vqs(vgdev->vdev);
err_vqs:
+ dev->dev_private = NULL;
kfree(vgdev);
return ret;
}
--
2.11.0

2021-05-19 17:47:10

by Gerd Hoffmann

Subject: Re: [PATCH 1/3] drm/virtio: Fixes a potential NULL pointer dereference on probe failure

On Mon, May 17, 2021 at 04:49:11PM +0800, Xie Yongji wrote:
> The dev->dev_private might not be allocated if virtio_gpu_pci_quirk()
> or virtio_gpu_init() failed. In this case, we should avoid the cleanup
> in virtio_gpu_release().

Pushed all three to drm-misc-next.

thanks,
Gerd