dev_get_drvdata simply returns the driver data part of drm_dev, which has its lifetime bound to the drm_dev. So only drop the reference in the error paths, on success they will get dropped later.
Cc: stable@vger.kernel.org Fixes: 9ba2556cef1df ("drm/mediatek: clean up driver data initialisation") Signed-off-by: Sjoerd Simons sjoerd@collabora.com --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 7841f59c52ee772dba3de416f410604f5f13eef2..c85fbecce9d6f0ade700e047e067ee7f9250f037 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -373,7 +373,7 @@ static int mtk_drm_match(struct device *dev, const void *data) static bool mtk_drm_get_all_drm_priv(struct device *dev) { struct mtk_drm_private *drm_priv = dev_get_drvdata(dev); - struct mtk_drm_private *all_drm_priv[MAX_CRTC]; + struct mtk_drm_private *all_drm_priv[MAX_CRTC] = { NULL, }; struct mtk_drm_private *temp_drm_priv; struct device_node *phandle = dev->parent->of_node; const struct of_device_id *of_id; @@ -399,16 +399,22 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev) continue;
temp_drm_priv = dev_get_drvdata(drm_dev); - put_device(drm_dev); - if (!temp_drm_priv) + if (!temp_drm_priv) { + put_device(drm_dev); continue; + }
- if (temp_drm_priv->data->main_len) + if (temp_drm_priv->data->main_len) { all_drm_priv[CRTC_MAIN] = temp_drm_priv; - else if (temp_drm_priv->data->ext_len) + } else if (temp_drm_priv->data->ext_len) { all_drm_priv[CRTC_EXT] = temp_drm_priv; - else if (temp_drm_priv->data->third_len) + } else if (temp_drm_priv->data->third_len) { all_drm_priv[CRTC_THIRD] = temp_drm_priv; + } else { + dev_warn(drm_dev, "Could not determine crtc type"); + put_device(drm_dev); + continue; + }
if (temp_drm_priv->mtk_drm_bound) cnt++; @@ -427,6 +433,10 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev) return true; }
+ for (i = 0; i < MAX_CRTC; i++) + if (all_drm_priv[i]) + put_device(all_drm_priv[i]->dev); + return false; }
--- base-commit: 22b5e8cde1db67c64b83fc0e4e1ab166cacff246 change-id: 20251002-mtk-drm-refcount-e0f718d9364a
Best regards,
Il 03/10/25 10:08, Sjoerd Simons ha scritto:
dev_get_drvdata simply returns the driver data part of drm_dev, which has its lifetime bound to the drm_dev. So only drop the reference in the error paths, on success they will get dropped later.
Cc: stable@vger.kernel.org Fixes: 9ba2556cef1df ("drm/mediatek: clean up driver data initialisation") Signed-off-by: Sjoerd Simons sjoerd@collabora.com
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
On Fri, Oct 03, 2025 at 10:08:28AM +0200, Sjoerd Simons wrote:
dev_get_drvdata simply returns the driver data part of drm_dev, which has its lifetime bound to the drm_dev.
No, that is not correct: the driver data goes away when the driver is unbound, not when the device is freed.
So only drop the reference in the error paths, on success they will get dropped later.
But there is indeed a partial fix for the device leak here which was overlooked. Good catch.
Cc: stable@vger.kernel.org Fixes: 9ba2556cef1df ("drm/mediatek: clean up driver data initialisation")
This is clearly not the commit that introduced the issue (even if I also failed to notice the reference imbalance).
Since there is no need to keep the references, I've just sent a fix which instead fixes this by dropping the previous partial fix:
https://lore.kernel.org/r/20251006093937.27869-1-johan@kernel.org
Johan
linux-stable-mirror@lists.linaro.org