[PATCH] fbdev: omapfb: panel-dsi-cm: initialize lock before registering display

[PATCH] fbdev: omapfb: panel-dsi-cm: initialize lock before registering display

[Runyu Xiao]

dsicm_probe() registers the display before initializing ddata->lock.
Once omapdss_register_display() publishes the display, another consumer
can reach a dsicm callback that takes this mutex while it is still
uninitialized.

Initialize the mutex before registering the display so the published
callbacks always see a valid lock.

Fixes: f76ee892a99e ("omapfb: copy omapdss & displays for omapfb")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
index 1d75f27c6b80..ce1d4d5f4034 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
@@ -1151,14 +1151,14 @@ static int dsicm_probe(struct platform_device *pdev)
 	dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE |
 		OMAP_DSS_DISPLAY_CAP_TEAR_ELIM;
 
+	mutex_init(&ddata->lock);
+
 	r = omapdss_register_display(dssdev);
 	if (r) {
 		dev_err(dev, "Failed to register panel\n");
 		goto err_reg;
 	}
 
-	mutex_init(&ddata->lock);
-
 	atomic_set(&ddata->do_update, 0);
 
 	ddata->reset_gpio = devm_gpiod_get(&pdev->dev, "reset", GPIOD_OUT_LOW);
-- 
2.34.1

Re: [PATCH] fbdev: omapfb: panel-dsi-cm: initialize lock before registering display

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] List corruption and Use-After-Free due to missing display unregistration on probe error paths.
- [High] Premature device registration exposes the display to consumers before initialization is complete, risking uninitialized resource access.
- [High] Use-After-Free during driver removal due to non-synchronous cancellation of delayed work.
- [High] Race condition between TE interrupt handler and timeout work causing double bus unlock.
--

--- Patch [1]: [PATCH] fbdev: omapfb: panel-dsi-cm: initialize lock before registering display ---
commit 62bd817117921c28fba7141ddc3cf344ad0e4b1e
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>

    fbdev: omapfb: panel-dsi-cm: initialize lock before registering display
    
    dsicm_probe() registers the display before initializing ddata->lock.
    Once omapdss_register_display() publishes the display, another consumer
    can reach a dsicm callback that takes this mutex while it is still
    uninitialized.
    
    Initialize the mutex before registering the display so the published
    callbacks always see a valid lock.

> diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
> index 5e7963b4aa93c..8d1dcd4371139 100644
> --- a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
> +++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
> @@ -1150,14 +1150,14 @@ static int dsicm_probe(struct platform_device *pdev)
>  	dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE |
>  		OMAP_DSS_DISPLAY_CAP_TEAR_ELIM;
>  
> +	mutex_init(&ddata->lock);
> +
>  	r = omapdss_register_display(dssdev);
>  	if (r) {
>  		dev_err(dev, "Failed to register panel\n");
>  		goto err_reg;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does publishing the display
globally here expose it to consumers before initialization is complete?

If a consumer like omapfb discovers the display immediately and calls
dsicm_update() or dsicm_disable(), could they trigger cancel_delayed_work()
or schedule_delayed_work() on ulps_work and te_timeout_work before those
works are initialized later in the probe function?

>  
> -	mutex_init(&ddata->lock);
> -
>  	atomic_set(&ddata->do_update, 0);
>  
>  	ddata->reset_gpio = devm_gpiod_get(&pdev->dev, "reset", GPIOD_OUT_LOW);

[Severity: High]
This is a pre-existing issue, but does the error path for this and subsequent
initializations leak the registered display?

If devm_gpiod_get() fails:

	if (r) {
		dev_err(&pdev->dev, "Failed to request reset gpio: %d\n", r);
		return r;
	}

the probe function returns the error directly, but omapdss_register_display()
has already added dssdev->panel_list to the global panel_list. Since
omapdss_unregister_display() is never called on these error paths, will this
leave a dangling pointer in the global list that could cause a panic if
traversed?

[Severity: High]
This isn't a bug introduced by this patch, but does driver removal have a
use-after-free risk?

In dsicm_remove():

	dsicm_cancel_ulps_work(ddata);

This uses cancel_delayed_work() for ulps_work, which does not wait for
currently executing work, and te_timeout_work cancellation appears to
be omitted entirely.

When remove() returns, devres frees the ddata structure. If either work item
is running or subsequently fires, will it dereference the freed ddata?

[Severity: High]
This is a pre-existing issue, but is there a race condition between the TE
interrupt handler and the timeout work?

In dsicm_te_timeout_work_callback():

	atomic_set(&ddata->do_update, 0);
	in->ops.dsi->bus_unlock(in);

If this executes concurrently with dsicm_te_isr(), could the ISR successfully
claim the update via atomic_cmpxchg() before the timeout work zeroes it?

If both paths then handle the frame, would this lead to an immediate
uncoordinated bus_unlock() from the timeout work and a subsequent
bus_unlock() from the frame-done callback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818135318.1957283-1-runyu.xiao@seu.edu.cn?part=1

Re: [PATCH] fbdev: omapfb: panel-dsi-cm: initialize lock before registering display

[Helge Deller]

On 8/18/26 15:53, Runyu Xiao wrote:
> dsicm_probe() registers the display before initializing ddata->lock.
> Once omapdss_register_display() publishes the display, another consumer
> can reach a dsicm callback that takes this mutex while it is still
> uninitialized.
> 
> Initialize the mutex before registering the display so the published
> callbacks always see a valid lock.
> 
> Fixes: f76ee892a99e ("omapfb: copy omapdss & displays for omapfb")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>   drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
applied.

Thanks!
Helge