media: atomisp: gc2235: fix UAF and memory leak

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>
Reviewed-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index d3414312e1de27c8264f4cdb7f8a147c8bf57f5f..998c9f46bd0684a6cf711679b6dda1436b9e00b0 100644 (file)
--- 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;
 }