| Message ID | 20210325212822.3663144-1-swboyd@chromium.org |
|---|---|
| State | Accepted |
| Commit | 5620b135aea49a8f41c86aaecfcb1598a7774121 |
| Headers | show |
| Series | drm/msm: Set drvdata to NULL when msm_drm_init() fails | expand |
Hello: This patch was applied to qcom/linux.git (refs/heads/for-next): On Thu, 25 Mar 2021 14:28:22 -0700 you wrote: > We should set the platform device's driver data to NULL here so that > code doesn't assume the struct drm_device pointer is valid when it could > have been destroyed. The lifetime of this pointer is managed by a kref > but when msm_drm_init() fails we call drm_dev_put() on the pointer which > will free the pointer's memory. This driver uses the component model, so > there's sort of two "probes" in this file, one for the platform device > i.e. msm_pdev_probe() and one for the component i.e. msm_drm_bind(). The > msm_drm_bind() code is using the platform device's driver data to store > struct drm_device so the two functions are intertwined. > > [...] Here is the summary with links: - drm/msm: Set drvdata to NULL when msm_drm_init() fails https://git.kernel.org/qcom/c/5620b135aea4 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index a5c6b8c23336..196907689c82 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -570,6 +570,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv) kfree(priv); err_put_drm_dev: drm_dev_put(ddev); + platform_set_drvdata(pdev, NULL); return ret; }
We should set the platform device's driver data to NULL here so that code doesn't assume the struct drm_device pointer is valid when it could have been destroyed. The lifetime of this pointer is managed by a kref but when msm_drm_init() fails we call drm_dev_put() on the pointer which will free the pointer's memory. This driver uses the component model, so there's sort of two "probes" in this file, one for the platform device i.e. msm_pdev_probe() and one for the component i.e. msm_drm_bind(). The msm_drm_bind() code is using the platform device's driver data to store struct drm_device so the two functions are intertwined. This relationship becomes a problem for msm_pdev_shutdown() when it tests the NULL-ness of the pointer to see if it should call drm_atomic_helper_shutdown(). The NULL test is a proxy check for if the pointer has been freed by kref_put(). If the drm_device has been destroyed, then we shouldn't call the shutdown helper, and we know that is the case if msm_drm_init() failed, therefore set the driver data to NULL so that this pointer liveness is tracked properly. Fixes: 9d5cbf5fe46e ("drm/msm: add shutdown support for display platform_driver") Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: Fabio Estevam <festevam@gmail.com> Cc: Krishna Manikandan <mkrishn@codeaurora.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/gpu/drm/msm/msm_drv.c | 1 + 1 file changed, 1 insertion(+)