| Message ID | 20230612220259.1884381-1-quic_bjorande@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dp: Free resources after unregistering them | expand |
On 13/06/2023 01:02, Bjorn Andersson wrote: > The DP component's unbind operation walks through the submodules to > unregister and clean things up. But if the unbind happens because the DP > controller itself is being removed, all the memory for those submodules > has just been freed. > > Change the order of these operations to avoid the many use-after-free > that otherwise happens in this code path. > > Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index bbb0550a022b..ebc84b8fddf8 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -1337,9 +1337,9 @@ static int dp_display_remove(struct platform_device *pdev) > { > struct dp_display_private *dp = dev_get_dp_display_private(&pdev->dev); > > + component_del(&pdev->dev, &dp_display_comp_ops); > dp_display_deinit_sub_modules(dp); > > - component_del(&pdev->dev, &dp_display_comp_ops); > platform_set_drvdata(pdev, NULL); This matches more or less the order in dp_display_probe(). Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> A note for the possible followup: the driver initializes DP debugfs from dpu_kms (ugh) by calling msm_dp_debugfs_init() -> dp_debug_get(). I think that dp_debug_put() in dp_display_deinit_sub_modules() does not look correct. > > return 0;
On Mon, 12 Jun 2023 15:02:59 -0700, Bjorn Andersson wrote: > The DP component's unbind operation walks through the submodules to > unregister and clean things up. But if the unbind happens because the DP > controller itself is being removed, all the memory for those submodules > has just been freed. > > Change the order of these operations to avoid the many use-after-free > that otherwise happens in this code path. > > [...] Applied, thanks! [1/1] drm/msm/dp: Free resources after unregistering them https://gitlab.freedesktop.org/lumag/msm/-/commit/fa0048a4b1fa Best regards,
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index bbb0550a022b..ebc84b8fddf8 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1337,9 +1337,9 @@ static int dp_display_remove(struct platform_device *pdev) { struct dp_display_private *dp = dev_get_dp_display_private(&pdev->dev); + component_del(&pdev->dev, &dp_display_comp_ops); dp_display_deinit_sub_modules(dp); - component_del(&pdev->dev, &dp_display_comp_ops); platform_set_drvdata(pdev, NULL); return 0;
The DP component's unbind operation walks through the submodules to unregister and clean things up. But if the unbind happens because the DP controller itself is being removed, all the memory for those submodules has just been freed. Change the order of these operations to avoid the many use-after-free that otherwise happens in this code path. Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support") Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> --- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)