Skip to content

Commit d57e67e

Browse files
Uuuuuuhogregkh
authored andcommitted
media: atomisp: gc2235: fix UAF and memory leak
[ Upstream commit 628f763 ] 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: a49d253 ("staging/atomisp: Add support for the Intel IPU v2") Signed-off-by: Yuho Choi <dbgh9129@gmail.com> Reviewed-by: Dan Carpenter <error27@gmail.com> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 2e3e4cc commit d57e67e

1 file changed

Lines changed: 18 additions & 11 deletions

File tree

drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ static int gc2235_probe(struct i2c_client *client)
809809

810810
ret = gc2235_s_config(&dev->sd, client->irq, gcpdev);
811811
if (ret)
812-
goto out_free;
812+
goto err_unregister_subdev;
813813

814814
dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
815815
dev->pad.flags = MEDIA_PAD_FL_SOURCE;
@@ -818,18 +818,16 @@ static int gc2235_probe(struct i2c_client *client)
818818
ret =
819819
v4l2_ctrl_handler_init(&dev->ctrl_handler,
820820
ARRAY_SIZE(gc2235_controls));
821-
if (ret) {
822-
gc2235_remove(client);
823-
return ret;
824-
}
821+
if (ret)
822+
goto err_csi_cfg;
825823

826824
for (i = 0; i < ARRAY_SIZE(gc2235_controls); i++)
827825
v4l2_ctrl_new_custom(&dev->ctrl_handler, &gc2235_controls[i],
828826
NULL);
829827

830828
if (dev->ctrl_handler.error) {
831-
gc2235_remove(client);
832-
return dev->ctrl_handler.error;
829+
ret = dev->ctrl_handler.error;
830+
goto err_ctrl_handler;
833831
}
834832

835833
/* Use same lock for controls as for everything else. */
@@ -838,14 +836,23 @@ static int gc2235_probe(struct i2c_client *client)
838836

839837
ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad);
840838
if (ret)
841-
gc2235_remove(client);
839+
goto err_ctrl_handler;
840+
841+
ret = atomisp_register_i2c_module(&dev->sd, gcpdev);
842+
if (ret)
843+
goto err_media_cleanup;
842844

843-
return atomisp_register_i2c_module(&dev->sd, gcpdev);
845+
return 0;
844846

845-
out_free:
847+
err_media_cleanup:
848+
media_entity_cleanup(&dev->sd.entity);
849+
err_ctrl_handler:
850+
v4l2_ctrl_handler_free(&dev->ctrl_handler);
851+
err_csi_cfg:
852+
dev->platform_data->csi_cfg(&dev->sd, 0);
853+
err_unregister_subdev:
846854
v4l2_device_unregister_subdev(&dev->sd);
847855
kfree(dev);
848-
849856
return ret;
850857
}
851858

0 commit comments

Comments
 (0)