mirror of https://lore.kernel.org/lkml/
 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	[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	[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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®