public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: imx8qxp-pxl2dpi: avoid of_node_put() on ERR_PTR()
@ 2026-04-19 12:21 Guangshuo Li
  2026-04-20  1:56 ` Frank Li
  0 siblings, 1 reply; 4+ messages in thread
From: Guangshuo Li @ 2026-04-19 12:21 UTC (permalink / raw)
  To: Liu Ying, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Luca Ceresoli, dri-devel, imx, linux-arm-kernel,
	linux-kernel
  Cc: Guangshuo Li, stable

imx8qxp_pxl2dpi_get_available_ep_from_port() may return ERR_PTR(-ENODEV)
or ERR_PTR(-EINVAL). imx8qxp_pxl2dpi_find_next_bridge() stores that
value in a __free(device_node) variable and then immediately checks
IS_ERR(ep).

On the error path, returning from the function triggers the cleanup
handler for __free(device_node). Since the device_node cleanup helper
only checks for NULL before calling of_node_put(), this results in
of_node_put(ERR_PTR(...)), which may lead to an invalid kobject_put()
dereference and crash the kernel.

Fix it by avoiding __free(device_node) for the endpoint pointer and
releasing it explicitly after obtaining the remote port parent.

This issue was found by a custom static analysis tool.

Fixes: ceea3f7806a10 ("drm/bridge: imx8qxp-pxl2dpi: simplify put of device_node pointers")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
index 441fd32dc91c..3610ca94a8e6 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
@@ -264,12 +264,15 @@ imx8qxp_pxl2dpi_get_available_ep_from_port(struct imx8qxp_pxl2dpi *p2d,
 
 static int imx8qxp_pxl2dpi_find_next_bridge(struct imx8qxp_pxl2dpi *p2d)
 {
-	struct device_node *ep __free(device_node) =
-		imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1);
+	struct device_node *ep;
+
+	ep = imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1);
 	if (IS_ERR(ep))
 		return PTR_ERR(ep);
 
 	struct device_node *remote __free(device_node) = of_graph_get_remote_port_parent(ep);
+	of_node_put(ep);
+
 	if (!remote || !of_device_is_available(remote)) {
 		DRM_DEV_ERROR(p2d->dev, "no available remote\n");
 		return -ENODEV;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/bridge: imx8qxp-pxl2dpi: avoid of_node_put() on ERR_PTR()
  2026-04-19 12:21 [PATCH] drm/bridge: imx8qxp-pxl2dpi: avoid of_node_put() on ERR_PTR() Guangshuo Li
@ 2026-04-20  1:56 ` Frank Li
  2026-04-20  2:19   ` Guangshuo Li
  0 siblings, 1 reply; 4+ messages in thread
From: Frank Li @ 2026-04-20  1:56 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Liu Ying, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Luca Ceresoli, dri-devel, imx, linux-arm-kernel,
	linux-kernel, stable

On Sun, Apr 19, 2026 at 08:21:34PM +0800, Guangshuo Li wrote:
> imx8qxp_pxl2dpi_get_available_ep_from_port() may return ERR_PTR(-ENODEV)
> or ERR_PTR(-EINVAL). imx8qxp_pxl2dpi_find_next_bridge() stores that
> value in a __free(device_node) variable and then immediately checks
> IS_ERR(ep).
>
> On the error path, returning from the function triggers the cleanup
> handler for __free(device_node). Since the device_node cleanup helper
> only checks for NULL before calling of_node_put(), this results in
> of_node_put(ERR_PTR(...)), which may lead to an invalid kobject_put()

Please fix
DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T))

If (!IS_ERR(_T))

Frank
> dereference and crash the kernel.
>
> Fix it by avoiding __free(device_node) for the endpoint pointer and
> releasing it explicitly after obtaining the remote port parent.
>
> This issue was found by a custom static analysis tool.
>
> Fixes: ceea3f7806a10 ("drm/bridge: imx8qxp-pxl2dpi: simplify put of device_node pointers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> index 441fd32dc91c..3610ca94a8e6 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
> @@ -264,12 +264,15 @@ imx8qxp_pxl2dpi_get_available_ep_from_port(struct imx8qxp_pxl2dpi *p2d,
>
>  static int imx8qxp_pxl2dpi_find_next_bridge(struct imx8qxp_pxl2dpi *p2d)
>  {
> -	struct device_node *ep __free(device_node) =
> -		imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1);
> +	struct device_node *ep;
> +
> +	ep = imx8qxp_pxl2dpi_get_available_ep_from_port(p2d, 1);
>  	if (IS_ERR(ep))
>  		return PTR_ERR(ep);
>
>  	struct device_node *remote __free(device_node) = of_graph_get_remote_port_parent(ep);
> +	of_node_put(ep);
> +
>  	if (!remote || !of_device_is_available(remote)) {
>  		DRM_DEV_ERROR(p2d->dev, "no available remote\n");
>  		return -ENODEV;
> --
> 2.43.0
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/bridge: imx8qxp-pxl2dpi: avoid of_node_put() on ERR_PTR()
  2026-04-20  1:56 ` Frank Li
@ 2026-04-20  2:19   ` Guangshuo Li
  2026-04-20  6:53     ` Liu Ying
  0 siblings, 1 reply; 4+ messages in thread
From: Guangshuo Li @ 2026-04-20  2:19 UTC (permalink / raw)
  To: Frank Li
  Cc: Liu Ying, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Luca Ceresoli, dri-devel, imx, linux-arm-kernel,
	linux-kernel, stable

Hi Frank,

Thanks for the review.

On Mon, 20 Apr 2026 at 09:56, Frank Li <Frank.li@nxp.com> wrote:
>
>
> Please fix
> DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T))
>
> If (!IS_ERR(_T))
>

You're right, fixing DEFINE_FREE(device_node, ...) is the proper way
to handle this:
if (_T && !IS_ERR(_T)) of_node_put(_T)

This is a better fix than handling it only in this driver.

I'll rework the patch based on your suggestion and send v2 later.

Thanks,
Guangshuo


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/bridge: imx8qxp-pxl2dpi: avoid of_node_put() on ERR_PTR()
  2026-04-20  2:19   ` Guangshuo Li
@ 2026-04-20  6:53     ` Liu Ying
  0 siblings, 0 replies; 4+ messages in thread
From: Liu Ying @ 2026-04-20  6:53 UTC (permalink / raw)
  To: Guangshuo Li, Frank Li
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Luca Ceresoli, dri-devel,
	imx, linux-arm-kernel, linux-kernel, stable

On Mon, Apr 20, 2026 at 10:19:35AM +0800, Guangshuo Li wrote:
> [You don't often get email from lgs201920130244@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hi Frank,
> 
> Thanks for the review.
> 
> On Mon, 20 Apr 2026 at 09:56, Frank Li <Frank.li@nxp.com> wrote:
>>
>>
>> Please fix
>> DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T))
>>
>> If (!IS_ERR(_T))
>>
> 
> You're right, fixing DEFINE_FREE(device_node, ...) is the proper way
> to handle this:
> if (_T && !IS_ERR(_T)) of_node_put(_T)

This would be intrusive because it effectively changes the cleanup action.
A similar case[1] was handled by ensuring only NULL pointer was returned
on error.  And, this is actually what i2c_of_probe_get_i2c_node()[2] does
now.

[1] https://lore.kernel.org/all/Zw-VkQ3di5nFHiXB@smile.fi.intel.com/
[2] https://elixir.bootlin.com/linux/v7.0/source/drivers/i2c/i2c-core-of-prober.c#L38-L58

BTW, even if the cleanup action needs to be changed, the 'if' condition
should be '!IS_ERR_OR_NULL(_T)'.

> 
> This is a better fix than handling it only in this driver.
> 
> I'll rework the patch based on your suggestion and send v2 later.
> 
> Thanks,
> Guangshuo

-- 
Regards,
Liu Ying


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-20  6:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-19 12:21 [PATCH] drm/bridge: imx8qxp-pxl2dpi: avoid of_node_put() on ERR_PTR() Guangshuo Li
2026-04-20  1:56 ` Frank Li
2026-04-20  2:19   ` Guangshuo Li
2026-04-20  6:53     ` Liu Ying

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox