[PATCH] media: msi2500: fix memory leak in msi2500_probe error path

Daiki Harada posted 1 patch 2 weeks, 6 days ago
There is a newer version of this series
drivers/media/usb/msi2500/msi2500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] media: msi2500: fix memory leak in msi2500_probe error path
Posted by Daiki Harada 2 weeks, 6 days ago
When video_register_device() fails in msi2500_probe(), the error path
jumps to err_unregister_v4l2_dev, which skips the call to
v4l2_ctrl_handler_free(). This leaks memory allocated by
v4l2_ctrl_handler_init() and v4l2_ctrl_add_handler().

Fix this by jumping to err_free_controls instead, which properly frees
the control handler before unregistering the v4l2 device.

Reported-by: syzbot+b1de0d5fd8a15fac11aa@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b1de0d5fd8a15fac11aa
Tested-by: syzbot+b1de0d5fd8a15fac11aa@syzkaller.appspotmail.com
Signed-off-by: Daiki Harada <daiky0325@gmail.com>
---
 drivers/media/usb/msi2500/msi2500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/msi2500/msi2500.c b/drivers/media/usb/msi2500/msi2500.c
index 1ff98956b680..76e1f2bfab0c 100644
--- a/drivers/media/usb/msi2500/msi2500.c
+++ b/drivers/media/usb/msi2500/msi2500.c
@@ -1265,7 +1265,7 @@ static int msi2500_probe(struct usb_interface *intf,
 	if (ret) {
 		dev_err(dev->dev,
 			"Failed to register as video device (%d)\n", ret);
-		goto err_unregister_v4l2_dev;
+		goto err_free_controls;
 	}
 	dev_info(dev->dev, "Registered as %s\n",
 		 video_device_node_name(&dev->vdev));
-- 
2.54.0
Re: [PATCH] media: msi2500: fix memory leak in msi2500_probe error path
Posted by Kohei Enju 2 weeks, 1 day ago
On Sun, 10 May 2026 01:57:55 +0000, Daiki Harada wrote:
> When video_register_device() fails in msi2500_probe(), the error path
> jumps to err_unregister_v4l2_dev, which skips the call to
> v4l2_ctrl_handler_free(). This leaks memory allocated by
> v4l2_ctrl_handler_init() and v4l2_ctrl_add_handler().
>
> Fix this by jumping to err_free_controls instead, which properly frees
> the control handler before unregistering the v4l2 device.
> 

Is the missing Fixes: tag intentional?

As far as I can tell from git blame, the Fixes tag might be:
Fixes: 2e68f841a5d1 ("[media] msi3101: use msi001 tuner driver")

That commit changed both the initialization sequence and the cleanup sequence on
failures, but didn't update the label it jumped to when video_register_device()
fails.

Otherwise, it looks good to me.

Side note: 
The code has changed quite a bit since the commit, including a function rename,
so I am not sure whether this can be cleanly backported to older stable trees.
Still, the Fixes tag might be helpful.

> Reported-by: syzbot+b1de0d5fd8a15fac11aa@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b1de0d5fd8a15fac11aa
> Tested-by: syzbot+b1de0d5fd8a15fac11aa@syzkaller.appspotmail.com
> Signed-off-by: Daiki Harada <daiky0325@gmail.com>
> ---
>  drivers/media/usb/msi2500/msi2500.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/msi2500/msi2500.c b/drivers/media/usb/msi2500/msi2500.c
> index 1ff98956b680..76e1f2bfab0c 100644
> --- a/drivers/media/usb/msi2500/msi2500.c
> +++ b/drivers/media/usb/msi2500/msi2500.c
> @@ -1265,7 +1265,7 @@ static int msi2500_probe(struct usb_interface *intf,
>  	if (ret) {
>  		dev_err(dev->dev,
>  			"Failed to register as video device (%d)\n", ret);
> -		goto err_unregister_v4l2_dev;
> +		goto err_free_controls;
>  	}
>  	dev_info(dev->dev, "Registered as %s\n",
>  		 video_device_node_name(&dev->vdev));