Commit 81443498 authored by Sakari Ailus's avatar Sakari Ailus Committed by Mauro Carvalho Chehab

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: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: default avatarHans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+samsung@kernel.org>
parent f4229204
...@@ -2006,6 +2006,8 @@ static int isp_remove(struct platform_device *pdev) ...@@ -2006,6 +2006,8 @@ static int isp_remove(struct platform_device *pdev)
media_entity_enum_cleanup(&isp->crashed); media_entity_enum_cleanup(&isp->crashed);
v4l2_async_notifier_cleanup(&isp->notifier); v4l2_async_notifier_cleanup(&isp->notifier);
kfree(isp);
return 0; return 0;
} }
...@@ -2196,7 +2198,7 @@ static int isp_probe(struct platform_device *pdev) ...@@ -2196,7 +2198,7 @@ static int isp_probe(struct platform_device *pdev)
int ret; int ret;
int i, m; int i, m;
isp = devm_kzalloc(&pdev->dev, sizeof(*isp), GFP_KERNEL); isp = kzalloc(sizeof(*isp), GFP_KERNEL);
if (!isp) { if (!isp) {
dev_err(&pdev->dev, "could not allocate memory\n"); dev_err(&pdev->dev, "could not allocate memory\n");
return -ENOMEM; return -ENOMEM;
...@@ -2205,17 +2207,19 @@ static int isp_probe(struct platform_device *pdev) ...@@ -2205,17 +2207,19 @@ static int isp_probe(struct platform_device *pdev)
ret = fwnode_property_read_u32(of_fwnode_handle(pdev->dev.of_node), ret = fwnode_property_read_u32(of_fwnode_handle(pdev->dev.of_node),
"ti,phy-type", &isp->phy_type); "ti,phy-type", &isp->phy_type);
if (ret) if (ret)
return ret; goto error_release_isp;
isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
"syscon"); "syscon");
if (IS_ERR(isp->syscon)) if (IS_ERR(isp->syscon)) {
return PTR_ERR(isp->syscon); ret = PTR_ERR(isp->syscon);
goto error_release_isp;
}
ret = of_property_read_u32_index(pdev->dev.of_node, ret = of_property_read_u32_index(pdev->dev.of_node,
"syscon", 1, &isp->syscon_offset); "syscon", 1, &isp->syscon_offset);
if (ret) if (ret)
return ret; goto error_release_isp;
isp->autoidle = autoidle; isp->autoidle = autoidle;
...@@ -2372,6 +2376,8 @@ static int isp_probe(struct platform_device *pdev) ...@@ -2372,6 +2376,8 @@ static int isp_probe(struct platform_device *pdev)
error: error:
v4l2_async_notifier_cleanup(&isp->notifier); v4l2_async_notifier_cleanup(&isp->notifier);
mutex_destroy(&isp->isp_mutex); mutex_destroy(&isp->isp_mutex);
error_release_isp:
kfree(isp);
return ret; return ret;
} }
......
...@@ -291,9 +291,10 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp) ...@@ -291,9 +291,10 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp)
{ {
struct ispstat *aewb = &isp->isp_aewb; struct ispstat *aewb = &isp->isp_aewb;
struct omap3isp_h3a_aewb_config *aewb_cfg; 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) if (!aewb_cfg)
return -ENOMEM; return -ENOMEM;
...@@ -303,12 +304,12 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp) ...@@ -303,12 +304,12 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp)
aewb->isp = isp; aewb->isp = isp;
/* Set recover state configuration */ /* Set recover state configuration */
aewb_recover_cfg = devm_kzalloc(isp->dev, sizeof(*aewb_recover_cfg), aewb_recover_cfg = kzalloc(sizeof(*aewb_recover_cfg), GFP_KERNEL);
GFP_KERNEL);
if (!aewb_recover_cfg) { if (!aewb_recover_cfg) {
dev_err(aewb->isp->dev, dev_err(aewb->isp->dev,
"AEWB: cannot allocate memory for recover configuration.\n"); "AEWB: cannot allocate memory for recover configuration.\n");
return -ENOMEM; ret = -ENOMEM;
goto err;
} }
aewb_recover_cfg->saturation_limit = OMAP3ISP_AEWB_MAX_SATURATION_LIM; aewb_recover_cfg->saturation_limit = OMAP3ISP_AEWB_MAX_SATURATION_LIM;
...@@ -325,13 +326,22 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp) ...@@ -325,13 +326,22 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp)
if (h3a_aewb_validate_params(aewb, aewb_recover_cfg)) { if (h3a_aewb_validate_params(aewb, aewb_recover_cfg)) {
dev_err(aewb->isp->dev, dev_err(aewb->isp->dev,
"AEWB: recover configuration is invalid.\n"); "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_cfg->buf_size = h3a_aewb_get_buf_size(aewb_recover_cfg);
aewb->recover_priv = 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;
} }
/* /*
......
...@@ -354,9 +354,10 @@ int omap3isp_h3a_af_init(struct isp_device *isp) ...@@ -354,9 +354,10 @@ int omap3isp_h3a_af_init(struct isp_device *isp)
{ {
struct ispstat *af = &isp->isp_af; struct ispstat *af = &isp->isp_af;
struct omap3isp_h3a_af_config *af_cfg; 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) if (af_cfg == NULL)
return -ENOMEM; return -ENOMEM;
...@@ -366,12 +367,12 @@ int omap3isp_h3a_af_init(struct isp_device *isp) ...@@ -366,12 +367,12 @@ int omap3isp_h3a_af_init(struct isp_device *isp)
af->isp = isp; af->isp = isp;
/* Set recover state configuration */ /* Set recover state configuration */
af_recover_cfg = devm_kzalloc(isp->dev, sizeof(*af_recover_cfg), af_recover_cfg = kzalloc(sizeof(*af_recover_cfg), GFP_KERNEL);
GFP_KERNEL);
if (!af_recover_cfg) { if (!af_recover_cfg) {
dev_err(af->isp->dev, dev_err(af->isp->dev,
"AF: cannot allocate memory for recover configuration.\n"); "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; af_recover_cfg->paxel.h_start = OMAP3ISP_AF_PAXEL_HZSTART_MIN;
...@@ -383,13 +384,22 @@ int omap3isp_h3a_af_init(struct isp_device *isp) ...@@ -383,13 +384,22 @@ int omap3isp_h3a_af_init(struct isp_device *isp)
if (h3a_af_validate_params(af, af_recover_cfg)) { if (h3a_af_validate_params(af, af_recover_cfg)) {
dev_err(af->isp->dev, dev_err(af->isp->dev,
"AF: recover configuration is invalid.\n"); "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_cfg->buf_size = h3a_af_get_buf_size(af_recover_cfg);
af->recover_priv = 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) void omap3isp_h3a_af_cleanup(struct isp_device *isp)
......
...@@ -478,9 +478,9 @@ int omap3isp_hist_init(struct isp_device *isp) ...@@ -478,9 +478,9 @@ int omap3isp_hist_init(struct isp_device *isp)
{ {
struct ispstat *hist = &isp->isp_hist; struct ispstat *hist = &isp->isp_hist;
struct omap3isp_hist_config *hist_cfg; 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) if (hist_cfg == NULL)
return -ENOMEM; return -ENOMEM;
...@@ -502,7 +502,7 @@ int omap3isp_hist_init(struct isp_device *isp) ...@@ -502,7 +502,7 @@ int omap3isp_hist_init(struct isp_device *isp)
if (IS_ERR(hist->dma_ch)) { if (IS_ERR(hist->dma_ch)) {
ret = PTR_ERR(hist->dma_ch); ret = PTR_ERR(hist->dma_ch);
if (ret == -EPROBE_DEFER) if (ret == -EPROBE_DEFER)
return ret; goto err;
hist->dma_ch = NULL; hist->dma_ch = NULL;
dev_warn(isp->dev, dev_warn(isp->dev,
...@@ -518,9 +518,12 @@ int omap3isp_hist_init(struct isp_device *isp) ...@@ -518,9 +518,12 @@ int omap3isp_hist_init(struct isp_device *isp)
hist->event_type = V4L2_EVENT_OMAP3ISP_HIST; hist->event_type = V4L2_EVENT_OMAP3ISP_HIST;
ret = omap3isp_stat_init(hist, "histogram", &hist_subdev_ops); ret = omap3isp_stat_init(hist, "histogram", &hist_subdev_ops);
err:
if (ret) { if (ret) {
if (hist->dma_ch) if (!IS_ERR_OR_NULL(hist->dma_ch))
dma_release_channel(hist->dma_ch); dma_release_channel(hist->dma_ch);
kfree(hist_cfg);
} }
return ret; return ret;
......
...@@ -1078,4 +1078,6 @@ void omap3isp_stat_cleanup(struct ispstat *stat) ...@@ -1078,4 +1078,6 @@ void omap3isp_stat_cleanup(struct ispstat *stat)
mutex_destroy(&stat->ioctl_lock); mutex_destroy(&stat->ioctl_lock);
isp_stat_bufs_free(stat); isp_stat_bufs_free(stat);
kfree(stat->buf); kfree(stat->buf);
kfree(stat->priv);
kfree(stat->recover_priv);
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment