All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] media: atomisp: ov2722: Return after failed pad initialization
@ 2026-09-15 14:08 Punnay Sharma
  2026-09-15 15:38 ` Dan Carpenter
  2026-09-16  7:36 ` [PATCH v2] media: atomisp: ov2722: Clean up resources on probe failure Punnay Sharma
  0 siblings, 2 replies; 3+ messages in thread
From: Punnay Sharma @ 2026-09-15 14:08 UTC (permalink / raw)
  To: Hans de Goede, Mauro Carvalho Chehab
  Cc: Greg Kroah-Hartman, Sakari Ailus, Andy Shevchenko, linux-media,
	linux-staging, linux-kernel

If media_entity_pads_init() fails, ov2722_probe() calls
ov2722_remove(), which frees the device structure. The probe then
continues into atomisp_register_i2c_module() with a pointer to the
freed subdevice, causing a use-after-free.

Return the original error immediately after cleanup to prevent
registration from accessing the freed device.

Signed-off-by: Punnay Sharma <punnaysharma805@gmail.com>
---
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index 2c41c496daa6..0748247ca42c 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -979,8 +979,10 @@ static int ov2722_probe(struct i2c_client *client)
 	dev->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 
 	ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad);
-	if (ret)
+	if (ret) {
 		ov2722_remove(client);
+		return ret;
+	}
 
 	return atomisp_register_i2c_module(&dev->sd, ovpdev);
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] media: atomisp: ov2722: Return after failed pad initialization
  2026-09-15 14:08 [PATCH v1] media: atomisp: ov2722: Return after failed pad initialization Punnay Sharma
@ 2026-09-15 15:38 ` Dan Carpenter
  2026-09-16  7:36 ` [PATCH v2] media: atomisp: ov2722: Clean up resources on probe failure Punnay Sharma
  1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2026-09-15 15:38 UTC (permalink / raw)
  To: Punnay Sharma
  Cc: Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Sakari Ailus, Andy Shevchenko, linux-media, linux-staging,
	linux-kernel

On Tue, Sep 15, 2026 at 07:38:28PM +0530, Punnay Sharma wrote:
> If media_entity_pads_init() fails, ov2722_probe() calls
> ov2722_remove(), which frees the device structure. The probe then
> continues into atomisp_register_i2c_module() with a pointer to the
> freed subdevice, causing a use-after-free.
> 
> Return the original error immediately after cleanup to prevent
> registration from accessing the freed device.
> 
> Signed-off-by: Punnay Sharma <punnaysharma805@gmail.com>

This needs a Fixes tag.

> ---
>  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> index 2c41c496daa6..0748247ca42c 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> @@ -979,8 +979,10 @@ static int ov2722_probe(struct i2c_client *client)
>  	dev->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>  
>  	ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad);
> -	if (ret)
> +	if (ret) {
>  		ov2722_remove(client);
> +		return ret;
> +	}
>  
>  	return atomisp_register_i2c_module(&dev->sd, ovpdev);

The bug you are describing is real and your patch fixes it, but
this isn't a complete fix.  For example, there is no cleanup if
atomisp_register_i2c_module() fails.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2] media: atomisp: ov2722: Clean up resources on probe failure
  2026-09-15 14:08 [PATCH v1] media: atomisp: ov2722: Return after failed pad initialization Punnay Sharma
  2026-09-15 15:38 ` Dan Carpenter
@ 2026-09-16  7:36 ` Punnay Sharma
  1 sibling, 0 replies; 3+ messages in thread
From: Punnay Sharma @ 2026-09-16  7:36 UTC (permalink / raw)
  To: Hans de Goede, Mauro Carvalho Chehab
  Cc: Dan Carpenter, Greg Kroah-Hartman, Sakari Ailus, Andy Shevchenko,
	linux-media, linux-staging, linux-kernel

ov2722_probe() mishandles cleanup on three failure paths:

- media_entity_pads_init(): calls ov2722_remove() but falls through
  to registration with a freed subdevice.
- atomisp_register_i2c_module(): returns without releasing the device,
  controls, CSI allocation or platform resources.
- __ov2722_init_ctrl_handler(): leaves the CSI allocation behind.

Route control-handler, pad initialization and module registration
failures to out_remove to call ov2722_remove(client) and return ret.
Keep the separate configuration failure path because ov2722_s_config()
already unwinds CSI setup.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Punnay Sharma <punnaysharma805@gmail.com>
---
v2:
  - Handle cleanup and error return if atomisp_register_i2c_module()
    fails (Dan Carpenter).
  - Route __ov2722_init_ctrl_handler() failure to out_remove to clean
    up CSI resources.
  - Add missing Fixes tag (Dan Carpenter).

 .../staging/media/atomisp/i2c/atomisp-ov2722.c    | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index 2c41c496daa6..272291e370fd 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -971,7 +971,7 @@ static int ov2722_probe(struct i2c_client *client)
 
 	ret = __ov2722_init_ctrl_handler(dev);
 	if (ret)
-		goto out_ctrl_handler_free;
+		goto out_remove;
 
 	dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	dev->pad.flags = MEDIA_PAD_FL_SOURCE;
@@ -980,12 +980,17 @@ static int ov2722_probe(struct i2c_client *client)
 
 	ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad);
 	if (ret)
-		ov2722_remove(client);
+		goto out_remove;
 
-	return atomisp_register_i2c_module(&dev->sd, ovpdev);
+	ret = atomisp_register_i2c_module(&dev->sd, ovpdev);
+	if (ret)
+		goto out_remove;
 
-out_ctrl_handler_free:
-	v4l2_ctrl_handler_free(&dev->ctrl_handler);
+	return 0;
+
+out_remove:
+	ov2722_remove(client);
+	return ret;
 
 out_free:
 	atomisp_gmin_remove_subdev(&dev->sd);

base-commit: 587858367581b9c55c3690f4e63382ad622719d4
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-16  7:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 14:08 [PATCH v1] media: atomisp: ov2722: Return after failed pad initialization Punnay Sharma
2026-09-15 15:38 ` Dan Carpenter
2026-09-16  7:36 ` [PATCH v2] media: atomisp: ov2722: Clean up resources on probe failure Punnay Sharma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.