media: omap3isp: Don't rely on devm for memory resource management

devm functions are fine for managing resources that are directly related
to the device at hand and that have no other dependencies. However, a
process holding a file handle to a device created by a driver for a device
may result in the file handle left behind after the device is long gone.
This will result in accessing released (and potentially reallocated)
memory.

Instead, manage the memory resources in the driver. Releasing the
resources can be later on bound to e.g. by releasing a reference.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index bd57174d81a781c41c801a3772c837745ae92499..5f30b802d319fccb3ca1bbaa428c929f9f72cff7 100644 (file)
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2006,6 +2006,8 @@ static int isp_remove(struct platform_device *pdev)
        media_entity_enum_cleanup(&isp->crashed);
        v4l2_async_notifier_cleanup(&isp->notifier);
 
+       kfree(isp);
+
        return 0;
 }
 
@@ -2196,7 +2198,7 @@ static int isp_probe(struct platform_device *pdev)
        int ret;
        int i, m;
 
-       isp = devm_kzalloc(&pdev->dev, sizeof(*isp), GFP_KERNEL);
+       isp = kzalloc(sizeof(*isp), GFP_KERNEL);
        if (!isp) {
                dev_err(&pdev->dev, "could not allocate memory\n");
                return -ENOMEM;
@@ -2205,17 +2207,19 @@ static int isp_probe(struct platform_device *pdev)
        ret = fwnode_property_read_u32(of_fwnode_handle(pdev->dev.of_node),
                                       "ti,phy-type", &isp->phy_type);
        if (ret)
-               return ret;
+               goto error_release_isp;
 
        isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
                                                      "syscon");
-       if (IS_ERR(isp->syscon))
-               return PTR_ERR(isp->syscon);
+       if (IS_ERR(isp->syscon)) {
+               ret = PTR_ERR(isp->syscon);
+               goto error_release_isp;
+       }
 
        ret = of_property_read_u32_index(pdev->dev.of_node,
                                         "syscon", 1, &isp->syscon_offset);
        if (ret)
-               return ret;
+               goto error_release_isp;
 
        isp->autoidle = autoidle;
 
@@ -2372,6 +2376,8 @@ error_isp:
 error:
        v4l2_async_notifier_cleanup(&isp->notifier);
        mutex_destroy(&isp->isp_mutex);
+error_release_isp:
+       kfree(isp);
 
        return ret;
 }

diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c b/drivers/media/platform/omap3isp/isph3a_aewb.c
index 3c82dea4d375f16c3b1712683ac4a55e80309fb1..2cefc30e7b18bada149d0d9ab928afbf9df2cbea 100644 (file)
--- a/drivers/media/platform/omap3isp/isph3a_aewb.c
+++ b/drivers/media/platform/omap3isp/isph3a_aewb.c
@@ -291,9 +291,10 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp)
 {
        struct ispstat *aewb = &isp->isp_aewb;
        struct omap3isp_h3a_aewb_config *aewb_cfg;
-       struct omap3isp_h3a_aewb_config *aewb_recover_cfg;
+       struct omap3isp_h3a_aewb_config *aewb_recover_cfg = NULL;
+       int ret;
 
-       aewb_cfg = devm_kzalloc(isp->dev, sizeof(*aewb_cfg), GFP_KERNEL);
+       aewb_cfg = kzalloc(sizeof(*aewb_cfg), GFP_KERNEL);
        if (!aewb_cfg)
                return -ENOMEM;
 
@@ -303,12 +304,12 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp)
        aewb->isp = isp;
 
        /* Set recover state configuration */
-       aewb_recover_cfg = devm_kzalloc(isp->dev, sizeof(*aewb_recover_cfg),
-                                       GFP_KERNEL);
+       aewb_recover_cfg = kzalloc(sizeof(*aewb_recover_cfg), GFP_KERNEL);
        if (!aewb_recover_cfg) {
                dev_err(aewb->isp->dev,
                        "AEWB: cannot allocate memory for recover configuration.\n");
-               return -ENOMEM;
+               ret = -ENOMEM;
+               goto err;
        }
 
        aewb_recover_cfg->saturation_limit = OMAP3ISP_AEWB_MAX_SATURATION_LIM;
@@ -325,13 +326,22 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp)
        if (h3a_aewb_validate_params(aewb, aewb_recover_cfg)) {
                dev_err(aewb->isp->dev,
                        "AEWB: recover configuration is invalid.\n");
-               return -EINVAL;
+               ret = -EINVAL;
+               goto err;
        }
 
        aewb_recover_cfg->buf_size = h3a_aewb_get_buf_size(aewb_recover_cfg);
        aewb->recover_priv = aewb_recover_cfg;
 
-       return omap3isp_stat_init(aewb, "AEWB", &h3a_aewb_subdev_ops);
+       ret = omap3isp_stat_init(aewb, "AEWB", &h3a_aewb_subdev_ops);
+
+err:
+       if (ret) {
+               kfree(aewb_cfg);
+               kfree(aewb_recover_cfg);
+       }
+
+       return ret;
 }
 
 /*

diff --git a/drivers/media/platform/omap3isp/isph3a_af.c b/drivers/media/platform/omap3isp/isph3a_af.c
index 4da25c84f0c6214337e61c2d08c00ec54c203705..843ec1dc5c9d15b752d24b5f641f4c51bbd6342e 100644 (file)
--- a/drivers/media/platform/omap3isp/isph3a_af.c
+++ b/drivers/media/platform/omap3isp/isph3a_af.c
@@ -354,9 +354,10 @@ int omap3isp_h3a_af_init(struct isp_device *isp)
 {
        struct ispstat *af = &isp->isp_af;
        struct omap3isp_h3a_af_config *af_cfg;
-       struct omap3isp_h3a_af_config *af_recover_cfg;
+       struct omap3isp_h3a_af_config *af_recover_cfg = NULL;
+       int ret;
 
-       af_cfg = devm_kzalloc(isp->dev, sizeof(*af_cfg), GFP_KERNEL);
+       af_cfg = kzalloc(sizeof(*af_cfg), GFP_KERNEL);
        if (af_cfg == NULL)
                return -ENOMEM;
 
@@ -366,12 +367,12 @@ int omap3isp_h3a_af_init(struct isp_device *isp)
        af->isp = isp;
 
        /* Set recover state configuration */
-       af_recover_cfg = devm_kzalloc(isp->dev, sizeof(*af_recover_cfg),
-                                     GFP_KERNEL);
+       af_recover_cfg = kzalloc(sizeof(*af_recover_cfg), GFP_KERNEL);
        if (!af_recover_cfg) {
                dev_err(af->isp->dev,
                        "AF: cannot allocate memory for recover configuration.\n");
-               return -ENOMEM;
+               ret = -ENOMEM;
+               goto err;
        }
 
        af_recover_cfg->paxel.h_start = OMAP3ISP_AF_PAXEL_HZSTART_MIN;
@@ -383,13 +384,22 @@ int omap3isp_h3a_af_init(struct isp_device *isp)
        if (h3a_af_validate_params(af, af_recover_cfg)) {
                dev_err(af->isp->dev,
                        "AF: recover configuration is invalid.\n");
-               return -EINVAL;
+               ret = -EINVAL;
+               goto err;
        }
 
        af_recover_cfg->buf_size = h3a_af_get_buf_size(af_recover_cfg);
        af->recover_priv = af_recover_cfg;
 
-       return omap3isp_stat_init(af, "AF", &h3a_af_subdev_ops);
+       ret = omap3isp_stat_init(af, "AF", &h3a_af_subdev_ops);
+
+err:
+       if (ret) {
+               kfree(af_cfg);
+               kfree(af_recover_cfg);
+       }
+
+       return ret;
 }
 
 void omap3isp_h3a_af_cleanup(struct isp_device *isp)

diff --git a/drivers/media/platform/omap3isp/isphist.c b/drivers/media/platform/omap3isp/isphist.c
index d4be3d0e06f94e5e15883af039d756fc244f40af..3b9ed808638782ec5282a9754d28e82dec27ba54 100644 (file)
--- a/drivers/media/platform/omap3isp/isphist.c
+++ b/drivers/media/platform/omap3isp/isphist.c
@@ -478,9 +478,9 @@ int omap3isp_hist_init(struct isp_device *isp)
 {
        struct ispstat *hist = &isp->isp_hist;
        struct omap3isp_hist_config *hist_cfg;
-       int ret = -1;
+       int ret;
 
-       hist_cfg = devm_kzalloc(isp->dev, sizeof(*hist_cfg), GFP_KERNEL);
+       hist_cfg = kzalloc(sizeof(*hist_cfg), GFP_KERNEL);
        if (hist_cfg == NULL)
                return -ENOMEM;
 
@@ -502,7 +502,7 @@ int omap3isp_hist_init(struct isp_device *isp)
                if (IS_ERR(hist->dma_ch)) {
                        ret = PTR_ERR(hist->dma_ch);
                        if (ret == -EPROBE_DEFER)
-                               return ret;
+                               goto err;
 
                        hist->dma_ch = NULL;
                        dev_warn(isp->dev,
@@ -518,9 +518,12 @@ int omap3isp_hist_init(struct isp_device *isp)
        hist->event_type = V4L2_EVENT_OMAP3ISP_HIST;
 
        ret = omap3isp_stat_init(hist, "histogram", &hist_subdev_ops);
+
+err:
        if (ret) {
-               if (hist->dma_ch)
+               if (!IS_ERR_OR_NULL(hist->dma_ch))
                        dma_release_channel(hist->dma_ch);
+               kfree(hist_cfg);
        }
 
        return ret;

diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
index 953a812bfe5e8d63fce212eadc5da3f300a49d5a..46a42c5dc1ccf968f52eb6c51e593aca1e156a69 100644 (file)
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -1078,4 +1078,6 @@ void omap3isp_stat_cleanup(struct ispstat *stat)
        mutex_destroy(&stat->ioctl_lock);
        isp_stat_bufs_free(stat);
        kfree(stat->buf);
+       kfree(stat->priv);
+       kfree(stat->recover_priv);
 }