dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Myeonghun Pak" <mhun512@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/panel: simple: Clean up resources on DSI attach failure
Date: Thu, 10 Sep 2026 22:47:18 +0000	[thread overview]
Message-ID: <20260910224719.15F871F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260910223803.8552-1-mhun512@gmail.com>

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

Pre-existing issues:
- [Critical] Use-After-Free (UAF) of the panel structure due to premature registration during probe
- [High] Unbalanced runtime PM and resource leak in panel_simple_remove()
- [High] TOCTOU race in panel_simple_shutdown() leading to lockless state corruption
--

commit 887f69784ef5d71614e53c7181f7e8b567905759
Author: Myeonghun Pak <mhun512@gmail.com>

drm/panel: simple: Clean up resources on DSI attach failure

This patch modifies panel_simple_dsi_probe() to call panel_simple_remove()
when mipi_dsi_attach() fails. This cleans up common probe resources instead
of just undoing panel registration.

> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index c95964ba25a95..767068de68160 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -6552,11 +6552,8 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
>  	dsi->lanes = desc->lanes;
>  
>  	err = mipi_dsi_attach(dsi);
> -	if (err) {
> -		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
> -
> -		drm_panel_remove(&panel->base);
> -	}
> +	if (err)
> +		panel_simple_remove(&dsi->dev);

[Severity: Critical]
This isn't a bug introduced by this patch, but could this error path lead to
a Use-After-Free (UAF) of the panel structure?

When panel_simple_dsi_probe() calls panel_simple_probe(), the panel is
allocated using devm_kzalloc() and registered globally via drm_panel_add().
At this point, it is visible to the system and a concurrent display driver
can discover it via of_drm_find_panel() and store a pointer to it.

If mipi_dsi_attach() fails here, panel_simple_remove() is called to
unregister it, and the probe function returns an error. Returning an error
triggers the driver core's devres cleanup, completely freeing the panel
structure memory. However, the concurrent display driver might still hold a
dangling pointer to this freed structure, resulting in a critical UAF upon
subsequent access (e.g., calling drm_panel_prepare()).

[Severity: High]
This is a pre-existing issue, but does calling panel_simple_remove() here
lead to an unbalanced runtime PM state and resource leak?

Looking at panel_simple_remove():

static void panel_simple_remove(struct device *dev)
{
    ...
    panel_simple_shutdown(dev);
    pm_runtime_dont_use_autosuspend(dev);
    pm_runtime_disable(dev);
    ...
}

panel_simple_shutdown() unprepares the panel, which calls
pm_runtime_put_autosuspend() to schedule an asynchronous suspend, but leaves
the device in the RPM_ACTIVE state. Then, pm_runtime_dont_use_autosuspend()
and pm_runtime_disable() cancel the pending autosuspend and disable runtime
PM entirely.

Because a synchronous suspend (e.g., pm_runtime_suspend or
pm_runtime_force_suspend) is omitted, the panel_simple_suspend() callback
never executes. Will this leave the panel's regulator and GPIO permanently
enabled, leading to a persistent hardware state and regulator enable count
leak?

[Severity: High]
This is also a pre-existing issue, but is there a TOCTOU race in
panel_simple_shutdown() (which is called by panel_simple_remove() above)?

In panel_simple_shutdown():

static void panel_simple_shutdown(struct device *dev)
{
    ...
    if (panel->base.enabled)
        drm_panel_disable(&panel->base);
    if (panel->base.prepared)
        drm_panel_unprepare(&panel->base);
    ...
}

These states are checked without holding any synchronization primitives
(such as DRM modeset locks). If this runs concurrently with a DRM display
controller performing a modeset (which independently calls
drm_panel_disable), both threads may observe panel->base.enabled == true
simultaneously.

Could this lockless race lead to double-disabling of the panel, unbalanced
regulator disables, and potential hardware state corruption?

>  
>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910223803.8552-1-mhun512@gmail.com?part=1

      reply	other threads:[~2026-09-10 22:47 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10 22:38 [PATCH] drm/panel: simple: Clean up resources on DSI attach failure Myeonghun Pak
2026-09-10 22:47 ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260910224719.15F871F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=mhun512@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox