drivers/media/platform/samsung/s5p-g2d/g2d.c | 3 +++ 1 file changed, 3 insertions(+)
g2d_probe() allocates a video_device with video_device_alloc() and
releases it from the rel_vdev error path if video_register_device()
fails.
This can double free the video_device when __video_register_device()
reaches device_register() and that call fails:
video_register_device()
-> __video_register_device()
-> device_register() fails
-> put_device(&vdev->dev)
-> v4l2_device_release()
-> vdev->release(vdev)
-> video_device_release(vdev)
g2d_probe()
-> rel_vdev
-> video_device_release(vfd)
Use video_device_release_empty() while registering the device so that
registration failure paths do not free vfd through vdev->release().
g2d_probe() then releases vfd exactly once from rel_vdev. Restore
video_device_release() after successful registration so the registered
device keeps its normal lifetime handling.
This issue was found by a static analysis tool I am developing.
Fixes: 918847341af0 ("[media] v4l: add G2D driver for s5p device family")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/media/platform/samsung/s5p-g2d/g2d.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/samsung/s5p-g2d/g2d.c b/drivers/media/platform/samsung/s5p-g2d/g2d.c
index a18b13db19d5..f38c28abd6d9 100644
--- a/drivers/media/platform/samsung/s5p-g2d/g2d.c
+++ b/drivers/media/platform/samsung/s5p-g2d/g2d.c
@@ -684,6 +684,7 @@ static int g2d_probe(struct platform_device *pdev)
goto unreg_v4l2_dev;
}
*vfd = g2d_videodev;
+ vfd->release = video_device_release_empty;
set_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags);
vfd->lock = &dev->mutex;
vfd->v4l2_dev = &dev->v4l2_dev;
@@ -711,6 +712,8 @@ static int g2d_probe(struct platform_device *pdev)
v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
goto free_m2m;
}
+
+ vfd->release = video_device_release;
video_set_drvdata(vfd, dev);
dev->vfd = vfd;
v4l2_info(&dev->v4l2_dev, "device registered as /dev/video%d\n",
--
2.43.0
It was 2026-05-17 nie 19:46, when Guangshuo Li wrote:
> g2d_probe() allocates a video_device with video_device_alloc() and
> releases it from the rel_vdev error path if video_register_device()
> fails.
>
> This can double free the video_device when __video_register_device()
> reaches device_register() and that call fails:
>
> video_register_device()
> -> __video_register_device()
> -> device_register() fails
> -> put_device(&vdev->dev)
> -> v4l2_device_release()
> -> vdev->release(vdev)
> -> video_device_release(vdev)
>
> g2d_probe()
> -> rel_vdev
> -> video_device_release(vfd)
>
> Use video_device_release_empty() while registering the device so that
> registration failure paths do not free vfd through vdev->release().
> g2d_probe() then releases vfd exactly once from rel_vdev. Restore
> video_device_release() after successful registration so the registered
> device keeps its normal lifetime handling.
>
> This issue was found by a static analysis tool I am developing.
Thank you for taking time to analyze our code. I was going to write
something along the lines Marek Szyprowski has laready written in this
thread https://lore.kernel.org/all/CANUHTR-RfWnoMRRAoDb6CPZsaiAYBLy61dd0P6TnKYgjNeBWpA@mail.gmail.com/
Do follow his advise with this one too.
> Fixes: 918847341af0 ("[media] v4l: add G2D driver for s5p device family")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/media/platform/samsung/s5p-g2d/g2d.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/platform/samsung/s5p-g2d/g2d.c b/drivers/media/platform/samsung/s5p-g2d/g2d.c
> index a18b13db19d5..f38c28abd6d9 100644
> --- a/drivers/media/platform/samsung/s5p-g2d/g2d.c
> +++ b/drivers/media/platform/samsung/s5p-g2d/g2d.c
> @@ -684,6 +684,7 @@ static int g2d_probe(struct platform_device *pdev)
> goto unreg_v4l2_dev;
> }
> *vfd = g2d_videodev;
> + vfd->release = video_device_release_empty;
> set_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags);
> vfd->lock = &dev->mutex;
> vfd->v4l2_dev = &dev->v4l2_dev;
> @@ -711,6 +712,8 @@ static int g2d_probe(struct platform_device *pdev)
> v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
> goto free_m2m;
> }
> +
> + vfd->release = video_device_release;
> video_set_drvdata(vfd, dev);
> dev->vfd = vfd;
> v4l2_info(&dev->v4l2_dev, "device registered as /dev/video%d\n",
--
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
© 2016 - 2026 Red Hat, Inc.