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.
Handle each failure path locally and unwind only the initialized
resources. Return success only after the full probe sequence completes.
Signed-off-by: Yuho Choi <yqc5929@psu.edu>
---
Changes since v1:
- Edited the commit message to be imperative mood
- Corrected the previous mangled patch
.../media/atomisp/i2c/atomisp-gc2235.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index d3414312e1de2..f4eb15d307fae 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -837,13 +837,24 @@ static int gc2235_probe(struct i2c_client *client)
dev->sd.ctrl_handler = &dev->ctrl_handler;
ret = media_entity_pads_init(&dev->sd.entity, 1, &dev->pad);
- if (ret)
- gc2235_remove(client);
+ if (ret) {
+ dev_err(&client->dev, "media_entity_pads_init failed\n");
+ goto err_free_ctrl;
+ }
+
+ ret = atomisp_register_i2c_module(&dev->sd, gcpdev);
+ if (ret) {
+ dev_err(&client->dev, "atomisp_register_i2c_module failed\n");
+ goto err_entity_cleanup;
+ }
- return atomisp_register_i2c_module(&dev->sd, gcpdev);
+ return 0;
+err_entity_cleanup:
+ media_entity_cleanup(&dev->sd.entity);
+err_free_ctrl:
+ v4l2_ctrl_handler_free(&dev->ctrl_handler);
out_free:
- v4l2_device_unregister_subdev(&dev->sd);
kfree(dev);
return ret;
--
2.50.1 (Apple Git-155)