2023-01-06 03:10:33

by Jiasheng Jiang

Subject: [PATCH] drm/msm/dsi: Add missing check for alloc_ordered_workqueue

Add check for the return value of alloc_ordered_workqueue as it may return
NULL pointer and cause NULL pointer dereference.
Moreover, change the "return ret" into "goto fail" in order to be
consistent with the others.

Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
Fixes: 32d3e0feccfe ("drm/msm: dsi: Use OPP API to set clk/perf state")
Signed-off-by: Jiasheng Jiang <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 89aadd3b3202..12239f628d5a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1944,19 +1944,19 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)

ret = devm_pm_opp_set_clkname(&pdev->dev, "byte");
if (ret)
- return ret;
+ goto fail;
/* OPP table is optional */
ret = devm_pm_opp_of_add_table(&pdev->dev);
if (ret && ret != -ENODEV) {
dev_err(&pdev->dev, "invalid OPP table in device tree\n");
- return ret;
+ goto fail;
}

msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
if (msm_host->irq < 0) {
ret = msm_host->irq;
dev_err(&pdev->dev, "failed to get irq: %d\n", ret);
- return ret;
+ goto fail;
}

/* do not autoenable, will be enabled later */
@@ -1966,7 +1966,7 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
if (ret < 0) {
dev_err(&pdev->dev, "failed to request IRQ%u: %d\n",
msm_host->irq, ret);
- return ret;
+ goto fail;
}

init_completion(&msm_host->dma_comp);
@@ -1977,6 +1977,11 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)

/* setup workqueue */
msm_host->workqueue = alloc_ordered_workqueue("dsi_drm_work", 0);
+ if (!msm_host->workqueue) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
INIT_WORK(&msm_host->err_work, dsi_err_worker);

msm_dsi->id = msm_host->id;
--
2.25.1

2023-01-08 23:07:57

by Dmitry Baryshkov

Subject: Re: [PATCH] drm/msm/dsi: Add missing check for alloc_ordered_workqueue

On 06/01/2023 04:56, Jiasheng Jiang wrote:
> Add check for the return value of alloc_ordered_workqueue as it may return
> NULL pointer and cause NULL pointer dereference.
> Moreover, change the "return ret" into "goto fail" in order to be
> consistent with the others.
>
> Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> Fixes: 32d3e0feccfe ("drm/msm: dsi: Use OPP API to set clk/perf state")
> Signed-off-by: Jiasheng Jiang <[email protected]>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 89aadd3b3202..12239f628d5a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1944,19 +1944,19 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
>
> ret = devm_pm_opp_set_clkname(&pdev->dev, "byte");
> if (ret)
> - return ret;
> + goto fail;

fail: is just 'return ret', so these changes are not required. Instead
the fail label can be dropped.

> /* OPP table is optional */
> ret = devm_pm_opp_of_add_table(&pdev->dev);
> if (ret && ret != -ENODEV) {
> dev_err(&pdev->dev, "invalid OPP table in device tree\n");
> - return ret;
> + goto fail;
> }
>
> msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> if (msm_host->irq < 0) {
> ret = msm_host->irq;
> dev_err(&pdev->dev, "failed to get irq: %d\n", ret);
> - return ret;
> + goto fail;
> }
>
> /* do not autoenable, will be enabled later */
> @@ -1966,7 +1966,7 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
> if (ret < 0) {
> dev_err(&pdev->dev, "failed to request IRQ%u: %d\n",
> msm_host->irq, ret);
> - return ret;
> + goto fail;
> }
>
> init_completion(&msm_host->dma_comp);
> @@ -1977,6 +1977,11 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
>
> /* setup workqueue */
> msm_host->workqueue = alloc_ordered_workqueue("dsi_drm_work", 0);
> + if (!msm_host->workqueue) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> INIT_WORK(&msm_host->err_work, dsi_err_worker);
>
> msm_dsi->id = msm_host->id;

--
With best wishes
Dmitry