[PATCH 0/2] drm/mediatek: fix potential OF node use-after-free

[PATCH 0/2] drm/mediatek: fix potential OF node use-after-free

[Johan Hovold]

A recent change (included in the drm pull request for 6.17-rc4) fixed a
device reference leak but also introduced a potential OF node
use-after-free.

This series fixes the new OF node reference imbalance and drops the
unnecessary gotos introduced by the broken fix, effectively reverting
that change in favour of the minimal fix I had previously posted here:

	https://lore.kernel.org/lkml/20250722092722.425-1-johan@kernel.org/

These should go into 6.17 which (soon) has the broken fix, which was
also marked for stable backport.

Johan


Johan Hovold (2):
  drm/mediatek: fix potential OF node use-after-free
  drm/mediatek: clean up driver data initialisation

 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

-- 
2.49.1

[PATCH 1/2] drm/mediatek: fix potential OF node use-after-free

[Johan Hovold]

The for_each_child_of_node() helper drops the reference it takes to each
node as it iterates over children and an explicit of_node_put() is only
needed when exiting the loop early.

Drop the recently introduced bogus additional reference count decrement
at each iteration that could potentially lead to a use-after-free.

Fixes: 1f403699c40f ("drm/mediatek: Fix device/node reference count leaks in mtk_drm_get_all_drm_priv")
Cc: Ma Ke <make24@iscas.ac.cn>
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 34131ae2c207..3b02ed0a16da 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -388,11 +388,11 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
 
 		of_id = of_match_node(mtk_drm_of_ids, node);
 		if (!of_id)
-			goto next_put_node;
+			continue;
 
 		pdev = of_find_device_by_node(node);
 		if (!pdev)
-			goto next_put_node;
+			continue;
 
 		drm_dev = device_find_child(&pdev->dev, NULL, mtk_drm_match);
 		if (!drm_dev)
@@ -418,11 +418,10 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
 next_put_device_pdev_dev:
 		put_device(&pdev->dev);
 
-next_put_node:
-		of_node_put(node);
-
-		if (cnt == MAX_CRTC)
+		if (cnt == MAX_CRTC) {
+			of_node_put(node);
 			break;
+		}
 	}
 
 	if (drm_priv->data->mmsys_dev_num == cnt) {
-- 
2.49.1

Re: [PATCH 1/2] drm/mediatek: fix potential OF node use-after-free

[CK Hu (胡俊光)]

On Fri, 2025-08-29 at 11:03 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> The for_each_child_of_node() helper drops the reference it takes to each
> node as it iterates over children and an explicit of_node_put() is only
> needed when exiting the loop early.
> 
> Drop the recently introduced bogus additional reference count decrement
> at each iteration that could potentially lead to a use-after-free.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Fixes: 1f403699c40f ("drm/mediatek: Fix device/node reference count leaks in mtk_drm_get_all_drm_priv")
> Cc: Ma Ke <make24@iscas.ac.cn>
> Cc: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 34131ae2c207..3b02ed0a16da 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -388,11 +388,11 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
> 
>                 of_id = of_match_node(mtk_drm_of_ids, node);
>                 if (!of_id)
> -                       goto next_put_node;
> +                       continue;
> 
>                 pdev = of_find_device_by_node(node);
>                 if (!pdev)
> -                       goto next_put_node;
> +                       continue;
> 
>                 drm_dev = device_find_child(&pdev->dev, NULL, mtk_drm_match);
>                 if (!drm_dev)
> @@ -418,11 +418,10 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
>  next_put_device_pdev_dev:
>                 put_device(&pdev->dev);
> 
> -next_put_node:
> -               of_node_put(node);
> -
> -               if (cnt == MAX_CRTC)
> +               if (cnt == MAX_CRTC) {
> +                       of_node_put(node);
>                         break;
> +               }
>         }
> 
>         if (drm_priv->data->mmsys_dev_num == cnt) {
> --
> 2.49.1
> 

Re: [PATCH 1/2] drm/mediatek: fix potential OF node use-after-free

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/plain, Size: 2203 bytes --]

On Fri, 2025-08-29 at 11:03 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> The for_each_child_of_node() helper drops the reference it takes to each
> node as it iterates over children and an explicit of_node_put() is only
> needed when exiting the loop early.
> 
> Drop the recently introduced bogus additional reference count decrement
> at each iteration that could potentially lead to a use-after-free.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Fixes: 1f403699c40f ("drm/mediatek: Fix device/node reference count leaks in mtk_drm_get_all_drm_priv")
> Cc: Ma Ke <make24@iscas.ac.cn>
> Cc: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 34131ae2c207..3b02ed0a16da 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -388,11 +388,11 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
> 
>                 of_id = of_match_node(mtk_drm_of_ids, node);
>                 if (!of_id)
> -                       goto next_put_node;
> +                       continue;
> 
>                 pdev = of_find_device_by_node(node);
>                 if (!pdev)
> -                       goto next_put_node;
> +                       continue;
> 
>                 drm_dev = device_find_child(&pdev->dev, NULL, mtk_drm_match);
>                 if (!drm_dev)
> @@ -418,11 +418,10 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
>  next_put_device_pdev_dev:
>                 put_device(&pdev->dev);
> 
> -next_put_node:
> -               of_node_put(node);
> -
> -               if (cnt == MAX_CRTC)
> +               if (cnt == MAX_CRTC) {
> +                       of_node_put(node);
>                         break;
> +               }
>         }
> 
>         if (drm_priv->data->mmsys_dev_num == cnt) {
> --
> 2.49.1
> 


[-- Attachment #2: Type: text/html, Size: 5485 bytes --]

[PATCH 2/2] drm/mediatek: clean up driver data initialisation

[Johan Hovold]

The platform and drm devices are only used to look up the drm device and
its driver data respectively when initialising the driver data during
bind().

Drop the reference counts as soon as they have been used to make the
code more readable.

Note that the crtc count is never incremented on lookup failures.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 3b02ed0a16da..33b83576af7e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -395,12 +395,14 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
 			continue;
 
 		drm_dev = device_find_child(&pdev->dev, NULL, mtk_drm_match);
+		put_device(&pdev->dev);
 		if (!drm_dev)
-			goto next_put_device_pdev_dev;
+			continue;
 
 		temp_drm_priv = dev_get_drvdata(drm_dev);
+		put_device(drm_dev);
 		if (!temp_drm_priv)
-			goto next_put_device_drm_dev;
+			continue;
 
 		if (temp_drm_priv->data->main_len)
 			all_drm_priv[CRTC_MAIN] = temp_drm_priv;
@@ -412,12 +414,6 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
 		if (temp_drm_priv->mtk_drm_bound)
 			cnt++;
 
-next_put_device_drm_dev:
-		put_device(drm_dev);
-
-next_put_device_pdev_dev:
-		put_device(&pdev->dev);
-
 		if (cnt == MAX_CRTC) {
 			of_node_put(node);
 			break;
-- 
2.49.1

Re: [PATCH 2/2] drm/mediatek: clean up driver data initialisation

[CK Hu (胡俊光)]

On Fri, 2025-08-29 at 11:03 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> The platform and drm devices are only used to look up the drm device and
> its driver data respectively when initialising the driver data during
> bind().
> 
> Drop the reference counts as soon as they have been used to make the
> code more readable.
> 
> Note that the crtc count is never incremented on lookup failures.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 3b02ed0a16da..33b83576af7e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -395,12 +395,14 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
>                         continue;
> 
>                 drm_dev = device_find_child(&pdev->dev, NULL, mtk_drm_match);
> +               put_device(&pdev->dev);
>                 if (!drm_dev)
> -                       goto next_put_device_pdev_dev;
> +                       continue;
> 
>                 temp_drm_priv = dev_get_drvdata(drm_dev);
> +               put_device(drm_dev);
>                 if (!temp_drm_priv)
> -                       goto next_put_device_drm_dev;
> +                       continue;
> 
>                 if (temp_drm_priv->data->main_len)
>                         all_drm_priv[CRTC_MAIN] = temp_drm_priv;
> @@ -412,12 +414,6 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
>                 if (temp_drm_priv->mtk_drm_bound)
>                         cnt++;
> 
> -next_put_device_drm_dev:
> -               put_device(drm_dev);
> -
> -next_put_device_pdev_dev:
> -               put_device(&pdev->dev);
> -
>                 if (cnt == MAX_CRTC) {
>                         of_node_put(node);
>                         break;
> --
> 2.49.1
> 

Re: [PATCH 2/2] drm/mediatek: clean up driver data initialisation

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]

On Fri, 2025-08-29 at 11:03 +0200, Johan Hovold wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> The platform and drm devices are only used to look up the drm device and
> its driver data respectively when initialising the driver data during
> bind().
> 
> Drop the reference counts as soon as they have been used to make the
> code more readable.
> 
> Note that the crtc count is never incremented on lookup failures.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 3b02ed0a16da..33b83576af7e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -395,12 +395,14 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
>                         continue;
> 
>                 drm_dev = device_find_child(&pdev->dev, NULL, mtk_drm_match);
> +               put_device(&pdev->dev);
>                 if (!drm_dev)
> -                       goto next_put_device_pdev_dev;
> +                       continue;
> 
>                 temp_drm_priv = dev_get_drvdata(drm_dev);
> +               put_device(drm_dev);
>                 if (!temp_drm_priv)
> -                       goto next_put_device_drm_dev;
> +                       continue;
> 
>                 if (temp_drm_priv->data->main_len)
>                         all_drm_priv[CRTC_MAIN] = temp_drm_priv;
> @@ -412,12 +414,6 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
>                 if (temp_drm_priv->mtk_drm_bound)
>                         cnt++;
> 
> -next_put_device_drm_dev:
> -               put_device(drm_dev);
> -
> -next_put_device_pdev_dev:
> -               put_device(&pdev->dev);
> -
>                 if (cnt == MAX_CRTC) {
>                         of_node_put(node);
>                         break;
> --
> 2.49.1
> 


[-- Attachment #2: Type: text/html, Size: 5626 bytes --]

Re: [PATCH 0/2] drm/mediatek: fix potential OF node use-after-free

[AngeloGioacchino Del Regno]

Il 29/08/25 11:03, Johan Hovold ha scritto:
> A recent change (included in the drm pull request for 6.17-rc4) fixed a
> device reference leak but also introduced a potential OF node
> use-after-free.
> 
> This series fixes the new OF node reference imbalance and drops the
> unnecessary gotos introduced by the broken fix, effectively reverting
> that change in favour of the minimal fix I had previously posted here:
> 
> 	https://lore.kernel.org/lkml/20250722092722.425-1-johan@kernel.org/
> 
> These should go into 6.17 which (soon) has the broken fix, which was
> also marked for stable backport.
> 
> Johan
> 
> 
> Johan Hovold (2):
>    drm/mediatek: fix potential OF node use-after-free
>    drm/mediatek: clean up driver data initialisation
> 
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c | 23 +++++++++--------------
>   1 file changed, 9 insertions(+), 14 deletions(-)
> 

The whole series is
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

*** URGENT ***

CK, please send this immediately, because [1] breaks *ALL MediaTek boards* making
them *unbootable*.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/mediatek?h=next-20250910&id=1f403699c40f0806a707a9a6eed3b8904224021a

This series fixes the situation and makes them boot again.

Regards,
Angelo