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

최유호
2026-04-15T16:15:38Z

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