[PATCH] media: meson: ge2d: avoid double free on video register failure

[PATCH] media: meson: ge2d: avoid double free on video register failure

[Guangshuo Li]

ge2d_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)

  ge2d_probe()
    -> rel_vdev
       -> video_device_release(ge2d->vfd)

Use video_device_release_empty() while registering the device so that
registration failure paths do not free ge2d->vfd through vdev->release().
ge2d_probe() then releases ge2d->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: 59a635327ca7 ("media: meson: Add M2M driver for the Amlogic GE2D Accelerator Unit")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/media/platform/amlogic/meson-ge2d/ge2d.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/amlogic/meson-ge2d/ge2d.c b/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
index c5dc03905ce0..b367169e6ad8 100644
--- a/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
+++ b/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
@@ -983,6 +983,7 @@ static int ge2d_probe(struct platform_device *pdev)
 	}
 
 	*vfd = ge2d_videodev;
+	vfd->release = video_device_release_empty;
 	vfd->lock = &ge2d->mutex;
 	vfd->v4l2_dev = &ge2d->v4l2_dev;
 
@@ -1005,6 +1006,7 @@ static int ge2d_probe(struct platform_device *pdev)
 
 	v4l2_info(&ge2d->v4l2_dev, "Registered %s as /dev/%s\n",
 		  vfd->name, video_device_node_name(vfd));
+	vfd->release = video_device_release;
 
 	return 0;
 
-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH] media: meson: ge2d: avoid double free on video register failure

[Guangshuo Li]

ge2d_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)

  ge2d_probe()
    -> rel_vdev
       -> video_device_release(ge2d->vfd)

Use video_device_release_empty() while registering the device so that
registration failure paths do not free ge2d->vfd through vdev->release().
ge2d_probe() then releases ge2d->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: 59a635327ca7 ("media: meson: Add M2M driver for the Amlogic GE2D Accelerator Unit")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/media/platform/amlogic/meson-ge2d/ge2d.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/amlogic/meson-ge2d/ge2d.c b/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
index c5dc03905ce0..b367169e6ad8 100644
--- a/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
+++ b/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
@@ -983,6 +983,7 @@ static int ge2d_probe(struct platform_device *pdev)
 	}
 
 	*vfd = ge2d_videodev;
+	vfd->release = video_device_release_empty;
 	vfd->lock = &ge2d->mutex;
 	vfd->v4l2_dev = &ge2d->v4l2_dev;
 
@@ -1005,6 +1006,7 @@ static int ge2d_probe(struct platform_device *pdev)
 
 	v4l2_info(&ge2d->v4l2_dev, "Registered %s as /dev/%s\n",
 		  vfd->name, video_device_node_name(vfd));
+	vfd->release = video_device_release;
 
 	return 0;
 
-- 
2.43.0

Re: [PATCH] media: meson: ge2d: avoid double free on video register failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch's workaround for a double free violates kobject lifecycle rules, introducing a Use-After-Free.
- [High] Pre-existing Use-After-Free due to device-managed (devm) memory lifecycle mismatch with asynchronous character device operations.
--

commit 7ed6ce0ea116cd8dcd5ff2e1837f5d339cda90fa
Author: Guangshuo Li <lgs201920130244@gmail.com>

media: meson: ge2d: avoid double free on video register failure

This commit attempts to fix a double free when video_register_device
fails by temporarily assigning video_device_release_empty to the video
device release callback. It restores the original callback if registration
succeeds.

> diff --git a/drivers/media/platform/amlogic/meson-ge2d/ge2d.c b/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
> index c5dc03905ce0f..b367169e6ad84 100644
> --- a/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
> +++ b/drivers/media/platform/amlogic/meson-ge2d/ge2d.c
> @@ -983,6 +983,7 @@ static int ge2d_probe(struct platform_device *pdev)
>  	}
>  
>  	*vfd = ge2d_videodev;
> +	vfd->release = video_device_release_empty;

Can this workaround introduce a use-after-free regression in the kobject core?

When __video_register_device fails inside device_register, it calls
put_device to drop the reference count. The driver core expects the release
callback to free the containing structure.

By temporarily substituting vfd->release with video_device_release_empty,
put_device will not free the memory synchronously. The driver then explicitly
frees the video_device in the error path via video_device_release.

However, put_device is not guaranteed to execute cleanup synchronously. If
asynchronous kobject release debugging is enabled, it schedules cleanup
asynchronously on a workqueue.

Explicitly freeing the video_device struct immediately after registration
fails could destroy the embedded device struct memory while the kobject core
still holds an asynchronous reference.

>  	vfd->lock = &ge2d->mutex;
>  	vfd->v4l2_dev = &ge2d->v4l2_dev;

Does the use of devm memory for the meson_ge2d struct create a lifecycle
mismatch with the video device?

The ge2d context is allocated via devm_kzalloc, which automatically frees
the memory when the driver unbinds. However, this context embeds fields
like v4l2_dev and mutex that are referenced by the video_device.

If a user process opens the video character device and holds the file
descriptor open, the video_device remains active even after the driver
unbinds and the devm memory is freed.

If the user later closes the file descriptor, the final reference drop
invokes v4l2_release, which dereferences vdev->v4l2_dev and attempts
to lock the freed ge2d->mutex, resulting in a use-after-free regression.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517115343.955015-1-lgs201920130244@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic