drm/mediatek: fix potential OF node use-after-free
Subsystems (4)
cover drm/mediatek: fix potential OF node use-after-free
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/[email protected]/
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(-)
Signature · 1 line
2.49.1
PATCH 1/2 drm/mediatek: fix potential OF node use-after-free
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.
--- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/gpu/drm/mediatek/mtk_drm_drv.c +5 -6
@@ -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) {
Signature · 1 line
2.49.1
Quoted 9 lines · Johan Hovold
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 explicitof_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.
Quoted 45 lines
Fixes 1f403699c40f ("drm/mediatek: Fix device/node reference count leaks in mtk_drm_get_all_drm_priv")Cc Ma Ke <[email protected]>Signed-off-by Johan Hovold <[email protected]>--- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)drivers/gpu/drm/mediatek/mtk_drm_drv.c +5
-7@@ -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
PATCH 2/2 drm/mediatek: clean up driver data initialisation
In reply to Johan HovoldThe 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.
--- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/gpu/drm/mediatek/mtk_drm_drv.c +4 -8
@@ -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 ;
Signature · 1 line
2.49.1
Re: [PATCH 2/2] drm/mediatek: clean up driver data initialisation
Quoted 11 lines · Johan Hovold
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.
Quoted 43 lines
Signed-off-by Johan Hovold <[email protected]>--- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/mediatek/mtk_drm_drv.c +4
-9@@ -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
Il 29/08/25 11:03, Johan Hovold ha scritto:
Quoted 16 lines
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/[email protected]/These should go into 6.17 which (soon) has the broken fix, which was also marked for stable backport.
Johan
Quoted 7 lines
Johan Hovold (2):
drm/mediatek: fix potential OF node use-after-free drm/mediatek: clean up driver data initialisationdrivers/gpu/drm/mediatek/mtk_drm_drv.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-)
The whole series is
*** URGENT ***
CK, please send this immediately, because [1] breaks *ALL MediaTek boards* making them *unbootable*.
This series fixes the situation and makes them boot again.
Regards,
Angelo