[PATCH v4] media: atomisp: gc2235: fix UAF and memory leak

[PATCH v4] media: atomisp: gc2235: fix UAF and memory leak

[Yuho Choi]

gc2235_probe() handles its error paths incorrectly.

If media_entity_pads_init() fails, gc2235_remove() is called, which
tears down the subdev and frees dev, but then still falls through to
atomisp_register_i2c_module(). This results in use-after-free.

If atomisp_register_i2c_module() fails, the media entity and control
handler are left initialized and dev is leaked.

gc2235_remove() unconditionally calls media_entity_cleanup() and
v4l2_ctrl_handler_free(), but these are not initialized at every
error path in gc2235_probe().

Replace gc2235_remove() calls in the probe error paths with explicit
unwind labels that free only the resources initialized at each point
of failure, in reverse order of initialization.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
Changes since v3:
- Replaced goto out_free/gc2235_remove() with explicit unwind labels
  to release only initialized resources at each failure point
- Replaced the "Fixes" tag with the original commit for the driver

Changes since v2:
- Replaced gc2235_remove() calls in remaining two error paths with
  goto labels to unwind only initialized resources
- Added Fixes tag

Changes since v1:
- Edited the commit message to be imperative mood
- Corrected the previous mangled patch

 .../media/atomisp/i2c/atomisp-gc2235.c        | 29 ++++++++++++-------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index d3414312e1de2..998c9f46bd068 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -809,7 +809,7 @@ static int gc2235_probe(struct i2c_client *client)
 
 	ret = gc2235_s_config(&dev->sd, client->irq, gcpdev);
 	if (ret)
-		goto out_free;
+		goto err_unregister_subdev;
 
 	dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	dev->pad.flags = MEDIA_PAD_FL_SOURCE;
@@ -818,18 +818,16 @@ static int gc2235_probe(struct i2c_client *client)
 	ret =
 	    v4l2_ctrl_handler_init(&dev->ctrl_handler,
 				   ARRAY_SIZE(gc2235_controls));
-	if (ret) {
-		gc2235_remove(client);
-		return ret;
-	}
+	if (ret)
+		goto err_csi_cfg;
 
 	for (i = 0; i < ARRAY_SIZE(gc2235_controls); i++)
 		v4l2_ctrl_new_custom(&dev->ctrl_handler, &gc2235_controls[i],
 				     NULL);
 
 	if (dev->ctrl_handler.error) {
-		gc2235_remove(client);
-		return dev->ctrl_handler.error;
+		ret = dev->ctrl_handler.error;
+		goto err_ctrl_handler;
 	}
 
 	/* Use same lock for controls as for everything else. */
@@ -838,14 +836,23 @@ static int gc2235_probe(struct i2c_client *client)
 
 	ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad);
 	if (ret)
-		gc2235_remove(client);
+		goto err_ctrl_handler;
+
+	ret = atomisp_register_i2c_module(&dev->sd, gcpdev);
+	if (ret)
+		goto err_media_cleanup;
 
-	return atomisp_register_i2c_module(&dev->sd, gcpdev);
+	return 0;
 
-out_free:
+err_media_cleanup:
+	media_entity_cleanup(&dev->sd.entity);
+err_ctrl_handler:
+	v4l2_ctrl_handler_free(&dev->ctrl_handler);
+err_csi_cfg:
+	dev->platform_data->csi_cfg(&dev->sd, 0);
+err_unregister_subdev:
 	v4l2_device_unregister_subdev(&dev->sd);
 	kfree(dev);
-
 	return ret;
 }
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v4] media: atomisp: gc2235: fix UAF and memory leak

[최유호]

Dear Reviewers,

I am following up on my recent patch submission, "[PATCH v4] media:
atomisp: gc2235: fix
UAF and memory leak."

Please let me know if you have had a chance to review it or if any
further changes are
required.

Best regards,
Yuho

On Thu, 2 Apr 2026 at 20:23, Yuho Choi <dbgh9129@gmail.com> wrote:
>
> gc2235_probe() handles its error paths incorrectly.
>
> If media_entity_pads_init() fails, gc2235_remove() is called, which
> tears down the subdev and frees dev, but then still falls through to
> atomisp_register_i2c_module(). This results in use-after-free.
>
> If atomisp_register_i2c_module() fails, the media entity and control
> handler are left initialized and dev is leaked.
>
> gc2235_remove() unconditionally calls media_entity_cleanup() and
> v4l2_ctrl_handler_free(), but these are not initialized at every
> error path in gc2235_probe().
>
> Replace gc2235_remove() calls in the probe error paths with explicit
> unwind labels that free only the resources initialized at each point
> of failure, in reverse order of initialization.
>
> Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
> ---
> Changes since v3:
> - Replaced goto out_free/gc2235_remove() with explicit unwind labels
>   to release only initialized resources at each failure point
> - Replaced the "Fixes" tag with the original commit for the driver
>
> Changes since v2:
> - Replaced gc2235_remove() calls in remaining two error paths with
>   goto labels to unwind only initialized resources
> - Added Fixes tag
>
> Changes since v1:
> - Edited the commit message to be imperative mood
> - Corrected the previous mangled patch
>
>  .../media/atomisp/i2c/atomisp-gc2235.c        | 29 ++++++++++++-------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> index d3414312e1de2..998c9f46bd068 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> @@ -809,7 +809,7 @@ static int gc2235_probe(struct i2c_client *client)
>
>         ret = gc2235_s_config(&dev->sd, client->irq, gcpdev);
>         if (ret)
> -               goto out_free;
> +               goto err_unregister_subdev;
>
>         dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>         dev->pad.flags = MEDIA_PAD_FL_SOURCE;
> @@ -818,18 +818,16 @@ static int gc2235_probe(struct i2c_client *client)
>         ret =
>             v4l2_ctrl_handler_init(&dev->ctrl_handler,
>                                    ARRAY_SIZE(gc2235_controls));
> -       if (ret) {
> -               gc2235_remove(client);
> -               return ret;
> -       }
> +       if (ret)
> +               goto err_csi_cfg;
>
>         for (i = 0; i < ARRAY_SIZE(gc2235_controls); i++)
>                 v4l2_ctrl_new_custom(&dev->ctrl_handler, &gc2235_controls[i],
>                                      NULL);
>
>         if (dev->ctrl_handler.error) {
> -               gc2235_remove(client);
> -               return dev->ctrl_handler.error;
> +               ret = dev->ctrl_handler.error;
> +               goto err_ctrl_handler;
>         }
>
>         /* Use same lock for controls as for everything else. */
> @@ -838,14 +836,23 @@ static int gc2235_probe(struct i2c_client *client)
>
>         ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad);
>         if (ret)
> -               gc2235_remove(client);
> +               goto err_ctrl_handler;
> +
> +       ret = atomisp_register_i2c_module(&dev->sd, gcpdev);
> +       if (ret)
> +               goto err_media_cleanup;
>
> -       return atomisp_register_i2c_module(&dev->sd, gcpdev);
> +       return 0;
>
> -out_free:
> +err_media_cleanup:
> +       media_entity_cleanup(&dev->sd.entity);
> +err_ctrl_handler:
> +       v4l2_ctrl_handler_free(&dev->ctrl_handler);
> +err_csi_cfg:
> +       dev->platform_data->csi_cfg(&dev->sd, 0);
> +err_unregister_subdev:
>         v4l2_device_unregister_subdev(&dev->sd);
>         kfree(dev);
> -
>         return ret;
>  }
>
> --
> 2.50.1 (Apple Git-155)
>

Re: [PATCH v4] media: atomisp: gc2235: fix UAF and memory leak

[Dan Carpenter]

On Thu, Apr 02, 2026 at 08:23:19PM -0400, Yuho Choi wrote:
> gc2235_probe() handles its error paths incorrectly.
> 
> If media_entity_pads_init() fails, gc2235_remove() is called, which
> tears down the subdev and frees dev, but then still falls through to
> atomisp_register_i2c_module(). This results in use-after-free.
> 
> If atomisp_register_i2c_module() fails, the media entity and control
> handler are left initialized and dev is leaked.
> 
> gc2235_remove() unconditionally calls media_entity_cleanup() and
> v4l2_ctrl_handler_free(), but these are not initialized at every
> error path in gc2235_probe().
> 
> Replace gc2235_remove() calls in the probe error paths with explicit
> unwind labels that free only the resources initialized at each point
> of failure, in reverse order of initialization.
> 
> Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
> ---

Thanks.  LGTM!

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter

Re: [PATCH v4] media: atomisp: gc2235: fix UAF and memory leak

[최유호]

Dear Dan,

Thanks for the review. I do appreciate the LGTM.

Best regards,
Yuho Choi

On Wed, 15 Apr 2026 at 12:23, Dan Carpenter <error27@gmail.com> wrote:
>
> On Thu, Apr 02, 2026 at 08:23:19PM -0400, Yuho Choi wrote:
> > gc2235_probe() handles its error paths incorrectly.
> >
> > If media_entity_pads_init() fails, gc2235_remove() is called, which
> > tears down the subdev and frees dev, but then still falls through to
> > atomisp_register_i2c_module(). This results in use-after-free.
> >
> > If atomisp_register_i2c_module() fails, the media entity and control
> > handler are left initialized and dev is leaked.
> >
> > gc2235_remove() unconditionally calls media_entity_cleanup() and
> > v4l2_ctrl_handler_free(), but these are not initialized at every
> > error path in gc2235_probe().
> >
> > Replace gc2235_remove() calls in the probe error paths with explicit
> > unwind labels that free only the resources initialized at each point
> > of failure, in reverse order of initialization.
> >
> > Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> > Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
> > ---
>
> Thanks.  LGTM!
>
> Reviewed-by: Dan Carpenter <error27@gmail.com>
>
> regards,
> dan carpenter
>