| Message ID | 20240217150228.5788-6-johan+linaro@kernel.org |
|---|---|
| State | Accepted |
| Commit | d2d7b8e88023b75320662c2305d61779ff060950 |
| Headers | show |
| Series | soc: qcom: pmic_glink_altmode: fix drm bridge use-after-free | expand |
On 17/02/2024 16:02, Johan Hovold wrote: > Due to a long-standing issue in driver core, drivers may not probe defer > after having registered child devices to avoid triggering a probe > deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of > -EPROBE_DEFER")). > > This could potentially also trigger a bug in the DRM bridge > implementation which does not expect bridges to go away even if device > links may avoid triggering this (when enabled). > > Move registration of the DRM aux bridge to after looking up clocks and > other resources. > > Note that PHY creation can in theory also trigger a probe deferral when > a 'phy' supply is used. This does not seem to affect the QMP PHY driver > but the PHY subsystem should be reworked to address this (i.e. by > separating initialisation and registration of the PHY). > > Fixes: 35921910bbd0 ("phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE") > Fixes: 1904c3f578dc ("phy: qcom-qmp-combo: Introduce drm_bridge") > Cc: stable@vger.kernel.org # 6.5 > Cc: Bjorn Andersson <quic_bjorande@quicinc.com> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index 1ad10110dd25..e19d6a084f10 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -3566,10 +3566,6 @@ static int qmp_combo_probe(struct platform_device *pdev) > if (ret) > return ret; > > - ret = drm_aux_bridge_register(dev); > - if (ret) > - return ret; > - > /* Check for legacy binding with child nodes. */ > usb_np = of_get_child_by_name(dev->of_node, "usb3-phy"); > if (usb_np) { > @@ -3589,6 +3585,10 @@ static int qmp_combo_probe(struct platform_device *pdev) > if (ret) > goto err_node_put; > > + ret = drm_aux_bridge_register(dev); > + if (ret) > + goto err_node_put; > + > pm_runtime_set_active(dev); > ret = devm_pm_runtime_enable(dev); > if (ret) Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
On Sat, Feb 17, 2024 at 04:02:27PM +0100, Johan Hovold wrote: > Due to a long-standing issue in driver core, drivers may not probe defer > after having registered child devices to avoid triggering a probe > deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of > -EPROBE_DEFER")). > > This could potentially also trigger a bug in the DRM bridge > implementation which does not expect bridges to go away even if device > links may avoid triggering this (when enabled). > > Move registration of the DRM aux bridge to after looking up clocks and > other resources. > > Note that PHY creation can in theory also trigger a probe deferral when > a 'phy' supply is used. This does not seem to affect the QMP PHY driver > but the PHY subsystem should be reworked to address this (i.e. by > separating initialisation and registration of the PHY). > > Fixes: 35921910bbd0 ("phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE") > Fixes: 1904c3f578dc ("phy: qcom-qmp-combo: Introduce drm_bridge") > Cc: stable@vger.kernel.org # 6.5 > Cc: Bjorn Andersson <quic_bjorande@quicinc.com> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Bjorn Andersson <andersson@kernel.org> Regards, Bjorn > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index 1ad10110dd25..e19d6a084f10 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -3566,10 +3566,6 @@ static int qmp_combo_probe(struct platform_device *pdev) > if (ret) > return ret; > > - ret = drm_aux_bridge_register(dev); > - if (ret) > - return ret; > - > /* Check for legacy binding with child nodes. */ > usb_np = of_get_child_by_name(dev->of_node, "usb3-phy"); > if (usb_np) { > @@ -3589,6 +3585,10 @@ static int qmp_combo_probe(struct platform_device *pdev) > if (ret) > goto err_node_put; > > + ret = drm_aux_bridge_register(dev); > + if (ret) > + goto err_node_put; > + > pm_runtime_set_active(dev); > ret = devm_pm_runtime_enable(dev); > if (ret) > -- > 2.43.0 >
On Sat, 17 Feb 2024 at 17:03, Johan Hovold <johan+linaro@kernel.org> wrote: > > Due to a long-standing issue in driver core, drivers may not probe defer > after having registered child devices to avoid triggering a probe > deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of > -EPROBE_DEFER")). > > This could potentially also trigger a bug in the DRM bridge > implementation which does not expect bridges to go away even if device > links may avoid triggering this (when enabled). > > Move registration of the DRM aux bridge to after looking up clocks and > other resources. > > Note that PHY creation can in theory also trigger a probe deferral when > a 'phy' supply is used. This does not seem to affect the QMP PHY driver > but the PHY subsystem should be reworked to address this (i.e. by > separating initialisation and registration of the PHY). > > Fixes: 35921910bbd0 ("phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE") > Fixes: 1904c3f578dc ("phy: qcom-qmp-combo: Introduce drm_bridge") > Cc: stable@vger.kernel.org # 6.5 > Cc: Bjorn Andersson <quic_bjorande@quicinc.com> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 17-02-24, 16:02, Johan Hovold wrote: > Due to a long-standing issue in driver core, drivers may not probe defer > after having registered child devices to avoid triggering a probe > deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of > -EPROBE_DEFER")). > > This could potentially also trigger a bug in the DRM bridge > implementation which does not expect bridges to go away even if device > links may avoid triggering this (when enabled). > > Move registration of the DRM aux bridge to after looking up clocks and > other resources. > > Note that PHY creation can in theory also trigger a probe deferral when > a 'phy' supply is used. This does not seem to affect the QMP PHY driver > but the PHY subsystem should be reworked to address this (i.e. by > separating initialisation and registration of the PHY). Acked-by: Vinod Koul <vkoul@kernel.org>
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c index 1ad10110dd25..e19d6a084f10 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c @@ -3566,10 +3566,6 @@ static int qmp_combo_probe(struct platform_device *pdev) if (ret) return ret; - ret = drm_aux_bridge_register(dev); - if (ret) - return ret; - /* Check for legacy binding with child nodes. */ usb_np = of_get_child_by_name(dev->of_node, "usb3-phy"); if (usb_np) { @@ -3589,6 +3585,10 @@ static int qmp_combo_probe(struct platform_device *pdev) if (ret) goto err_node_put; + ret = drm_aux_bridge_register(dev); + if (ret) + goto err_node_put; + pm_runtime_set_active(dev); ret = devm_pm_runtime_enable(dev); if (ret)
Due to a long-standing issue in driver core, drivers may not probe defer after having registered child devices to avoid triggering a probe deferral loop (see fbc35b45f9f6 ("Add documentation on meaning of -EPROBE_DEFER")). This could potentially also trigger a bug in the DRM bridge implementation which does not expect bridges to go away even if device links may avoid triggering this (when enabled). Move registration of the DRM aux bridge to after looking up clocks and other resources. Note that PHY creation can in theory also trigger a probe deferral when a 'phy' supply is used. This does not seem to affect the QMP PHY driver but the PHY subsystem should be reworked to address this (i.e. by separating initialisation and registration of the PHY). Fixes: 35921910bbd0 ("phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE") Fixes: 1904c3f578dc ("phy: qcom-qmp-combo: Introduce drm_bridge") Cc: stable@vger.kernel.org # 6.5 Cc: Bjorn Andersson <quic_bjorande@quicinc.com> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)