All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-09-13  8:53 ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold

The MSM DRM driver is currently broken in multiple ways with respect to
probe deferral. Not only does the driver currently fail to probe again
after a late deferral, but due to a related use-after-free bug this also
triggers NULL-pointer dereferences.

These bugs are not new but have become critical with the release of
5.19 where probe is deferred in case the aux-bus EP panel driver has not
yet been loaded.

The underlying problem is lifetime issues due to careless use of
device-managed resources.

Specifically, device-managed resources allocated post component bind
must be tied to the lifetime of the aggregate DRM device or they will
not necessarily be released when binding of the aggregate device is
deferred.

The following call chain and pseudo code serves as an illustration of
the problem:

 - platform_probe(pdev1)
   - dp_display_probe()
     - component_add()

 - platform_probe(pdev2) 				// last component
   - dp_display_probe()					// d0
       - component_add()
         - try_to_bring_up_aggregate_device()
	   - devres_open_group(adev->parent)		// d1

	   - msm_drm_bind()
	     - msm_drm_init()
	       - component_bind_all()
	         - for_each_component()
		   - component_bind()
		     - devres_open_group(&pdev->dev)	// d2
	             - dp_display_bind()
		       - devm_kzalloc(&pdev->dev)	// a1, OK
		     - devres_close_group(&pdev->dev)	// d3

	       - dpu_kms_hw_init()
	         - for_each_panel()
	           - msm_dp_modeset_init()
		     - dp_display_request_irq()
		       - devm_request_irq(&pdev->dev)	// a2, BUG
		     - if (pdev == pdev2 && condition)
		       - return -EPROBE_DEFER;

	      - if (error)
 	        - component_unbind_all()
	          - for_each_component()
		    - component_unbind()
		      - dp_display_unbind()
		      - devres_release_group(&pdev->dev) // d4, only a1 is freed

           - if (error)
	     - devres_release_group(adev->parent)	// d5

The device-managed allocation a2 is buggy as its lifetime is tied to the
component platform device and will not be released when the aggregate
device bind fails (e.g. due to a probe deferral).

When pdev2 is later probed again, the attempt to allocate the IRQ a
second time will fail for pdev1 (which is still bound to its platform
driver).

This series fixes the lifetime issues by tying the lifetime of a2 (and
similar allocations) to the lifetime of the aggregate device so that a2
is released at d5.

In some cases, such has for the DP IRQ, the above situation can also be
avoided by moving the allocation in question to the platform driver
probe (d0) or component bind (between d2 and d3). But as doing so is not
a general fix, this can be done later as a cleanup/optimisation.

Johan

Changes in v2
 - use a custom devres action instead of amending the AUX bus interface
   (Doug)
 - split sanity check fixes and cleanups per bridge type (Dmitry)
 - add another Fixes tag for the missing bridge counter reset (Dmitry)


Johan Hovold (10):
  drm/msm: fix use-after-free on probe deferral
  drm/msm/dp: fix memory corruption with too many bridges
  drm/msm/dsi: fix memory corruption with too many bridges
  drm/msm/hdmi: fix memory corruption with too many bridges
  drm/msm/dp: fix IRQ lifetime
  drm/msm/dp: fix aux-bus EP lifetime
  drm/msm/dp: fix bridge lifetime
  drm/msm/hdmi: fix IRQ lifetime
  drm/msm/dp: drop modeset sanity checks
  drm/msm/dsi: drop modeset sanity checks

 drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
 drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
 drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
 drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
 drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
 drivers/gpu/drm/msm/msm_drv.c       |  1 +
 6 files changed, 37 insertions(+), 17 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 87+ messages in thread

* [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-09-13  8:53 ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

The MSM DRM driver is currently broken in multiple ways with respect to
probe deferral. Not only does the driver currently fail to probe again
after a late deferral, but due to a related use-after-free bug this also
triggers NULL-pointer dereferences.

These bugs are not new but have become critical with the release of
5.19 where probe is deferred in case the aux-bus EP panel driver has not
yet been loaded.

The underlying problem is lifetime issues due to careless use of
device-managed resources.

Specifically, device-managed resources allocated post component bind
must be tied to the lifetime of the aggregate DRM device or they will
not necessarily be released when binding of the aggregate device is
deferred.

The following call chain and pseudo code serves as an illustration of
the problem:

 - platform_probe(pdev1)
   - dp_display_probe()
     - component_add()

 - platform_probe(pdev2) 				// last component
   - dp_display_probe()					// d0
       - component_add()
         - try_to_bring_up_aggregate_device()
	   - devres_open_group(adev->parent)		// d1

	   - msm_drm_bind()
	     - msm_drm_init()
	       - component_bind_all()
	         - for_each_component()
		   - component_bind()
		     - devres_open_group(&pdev->dev)	// d2
	             - dp_display_bind()
		       - devm_kzalloc(&pdev->dev)	// a1, OK
		     - devres_close_group(&pdev->dev)	// d3

	       - dpu_kms_hw_init()
	         - for_each_panel()
	           - msm_dp_modeset_init()
		     - dp_display_request_irq()
		       - devm_request_irq(&pdev->dev)	// a2, BUG
		     - if (pdev == pdev2 && condition)
		       - return -EPROBE_DEFER;

	      - if (error)
 	        - component_unbind_all()
	          - for_each_component()
		    - component_unbind()
		      - dp_display_unbind()
		      - devres_release_group(&pdev->dev) // d4, only a1 is freed

           - if (error)
	     - devres_release_group(adev->parent)	// d5

The device-managed allocation a2 is buggy as its lifetime is tied to the
component platform device and will not be released when the aggregate
device bind fails (e.g. due to a probe deferral).

When pdev2 is later probed again, the attempt to allocate the IRQ a
second time will fail for pdev1 (which is still bound to its platform
driver).

This series fixes the lifetime issues by tying the lifetime of a2 (and
similar allocations) to the lifetime of the aggregate device so that a2
is released at d5.

In some cases, such has for the DP IRQ, the above situation can also be
avoided by moving the allocation in question to the platform driver
probe (d0) or component bind (between d2 and d3). But as doing so is not
a general fix, this can be done later as a cleanup/optimisation.

Johan

Changes in v2
 - use a custom devres action instead of amending the AUX bus interface
   (Doug)
 - split sanity check fixes and cleanups per bridge type (Dmitry)
 - add another Fixes tag for the missing bridge counter reset (Dmitry)


Johan Hovold (10):
  drm/msm: fix use-after-free on probe deferral
  drm/msm/dp: fix memory corruption with too many bridges
  drm/msm/dsi: fix memory corruption with too many bridges
  drm/msm/hdmi: fix memory corruption with too many bridges
  drm/msm/dp: fix IRQ lifetime
  drm/msm/dp: fix aux-bus EP lifetime
  drm/msm/dp: fix bridge lifetime
  drm/msm/hdmi: fix IRQ lifetime
  drm/msm/dp: drop modeset sanity checks
  drm/msm/dsi: drop modeset sanity checks

 drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
 drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
 drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
 drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
 drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
 drivers/gpu/drm/msm/msm_drv.c       |  1 +
 6 files changed, 37 insertions(+), 17 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 87+ messages in thread

* [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

The bridge counter was never reset when tearing down the DRM device so
that stale pointers to deallocated structures would be accessed on the
next tear down (e.g. after a second late bind deferral).

Given enough bridges and a few probe deferrals this could currently also
lead to data beyond the bridge array being corrupted.

Fixes: d28ea556267c ("drm/msm: properly add and remove internal bridges")
Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
Cc: stable@vger.kernel.org      # 3.12
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/msm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 391d86b54ded..d254fe2507ec 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)
 
 	for (i = 0; i < priv->num_bridges; i++)
 		drm_bridge_remove(priv->bridges[i]);
+	priv->num_bridges = 0;
 
 	pm_runtime_get_sync(dev);
 	msm_irq_uninstall(ddev);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

The bridge counter was never reset when tearing down the DRM device so
that stale pointers to deallocated structures would be accessed on the
next tear down (e.g. after a second late bind deferral).

Given enough bridges and a few probe deferrals this could currently also
lead to data beyond the bridge array being corrupted.

Fixes: d28ea556267c ("drm/msm: properly add and remove internal bridges")
Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
Cc: stable@vger.kernel.org      # 3.12
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/msm_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 391d86b54ded..d254fe2507ec 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)
 
 	for (i = 0; i < priv->num_bridges; i++)
 		drm_bridge_remove(priv->bridges[i]);
+	priv->num_bridges = 0;
 
 	pm_runtime_get_sync(dev);
 	msm_irq_uninstall(ddev);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

Add the missing sanity check on the bridge counter to avoid corrupting
data beyond the fixed-sized bridge array in case there are ever more
than eight bridges.

Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
Cc: stable@vger.kernel.org	# 5.17
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 3e284fed8d30..fbe950edaefe 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1604,6 +1604,12 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 		return -EINVAL;
 
 	priv = dev->dev_private;
+
+	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
+		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
+		return -ENOSPC;
+	}
+
 	dp_display->drm_dev = dev;
 
 	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Add the missing sanity check on the bridge counter to avoid corrupting
data beyond the fixed-sized bridge array in case there are ever more
than eight bridges.

Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
Cc: stable@vger.kernel.org	# 5.17
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 3e284fed8d30..fbe950edaefe 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1604,6 +1604,12 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 		return -EINVAL;
 
 	priv = dev->dev_private;
+
+	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
+		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
+		return -ENOSPC;
+	}
+
 	dp_display->drm_dev = dev;
 
 	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

Add the missing sanity check on the bridge counter to avoid corrupting
data beyond the fixed-sized bridge array in case there are ever more
than eight bridges.

Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
Cc: stable@vger.kernel.org	# 4.1
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 39bbabb5daf6..8a95c744972a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -218,6 +218,12 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
 		return -EINVAL;
 
 	priv = dev->dev_private;
+
+	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
+		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
+		return -ENOSPC;
+	}
+
 	msm_dsi->dev = dev;
 
 	ret = msm_dsi_host_modeset_init(msm_dsi->host, dev);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Add the missing sanity check on the bridge counter to avoid corrupting
data beyond the fixed-sized bridge array in case there are ever more
than eight bridges.

Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
Cc: stable@vger.kernel.org	# 4.1
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 39bbabb5daf6..8a95c744972a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -218,6 +218,12 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
 		return -EINVAL;
 
 	priv = dev->dev_private;
+
+	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
+		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
+		return -ENOSPC;
+	}
+
 	msm_dsi->dev = dev;
 
 	ret = msm_dsi_host_modeset_init(msm_dsi->host, dev);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

Add the missing sanity check on the bridge counter to avoid corrupting
data beyond the fixed-sized bridge array in case there are ever more
than eight bridges.

Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
Cc: stable@vger.kernel.org	# 3.12
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 93fe61b86967..a0ed6aa8e4e1 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -300,6 +300,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 	struct platform_device *pdev = hdmi->pdev;
 	int ret;
 
+	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
+		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
+		return -ENOSPC;
+	}
+
 	hdmi->dev = dev;
 	hdmi->encoder = encoder;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Add the missing sanity check on the bridge counter to avoid corrupting
data beyond the fixed-sized bridge array in case there are ever more
than eight bridges.

Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
Cc: stable@vger.kernel.org	# 3.12
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 93fe61b86967..a0ed6aa8e4e1 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -300,6 +300,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 	struct platform_device *pdev = hdmi->pdev;
 	int ret;
 
+	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
+		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
+		return -ENOSPC;
+	}
+
 	hdmi->dev = dev;
 	hdmi->encoder = encoder;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 05/10] drm/msm/dp: fix IRQ lifetime
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This is specifically true for the DP IRQ, which will otherwise remain
requested so that the next bind attempt fails when requesting the IRQ a
second time.

Since commit c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
this can happen when the aux-bus panel driver has not yet been loaded so
that probe is deferred.

Fix this by tying the device-managed lifetime of the DP IRQ to the DRM
device so that it is released when bind fails.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Cc: stable@vger.kernel.org      # 5.10
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 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 fbe950edaefe..ba557328710a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1258,7 +1258,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
 		return -EINVAL;
 	}
 
-	rc = devm_request_irq(&dp->pdev->dev, dp->irq,
+	rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
 			dp_display_irq_handler,
 			IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
 	if (rc < 0) {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 05/10] drm/msm/dp: fix IRQ lifetime
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This is specifically true for the DP IRQ, which will otherwise remain
requested so that the next bind attempt fails when requesting the IRQ a
second time.

Since commit c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
this can happen when the aux-bus panel driver has not yet been loaded so
that probe is deferred.

Fix this by tying the device-managed lifetime of the DP IRQ to the DRM
device so that it is released when bind fails.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Cc: stable@vger.kernel.org      # 5.10
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 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 fbe950edaefe..ba557328710a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1258,7 +1258,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
 		return -EINVAL;
 	}
 
-	rc = devm_request_irq(&dp->pdev->dev, dp->irq,
+	rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
 			dp_display_irq_handler,
 			IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
 	if (rc < 0) {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This can lead resource leaks or failure to bind the aggregate device
when binding is later retried and a second attempt to allocate the
resources is made.

For the DP aux-bus, an attempt to populate the bus a second time will
simply fail ("DP AUX EP device already populated").

Fix this by tying the lifetime of the EP device to the DRM device rather
than DP controller platform device.

Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
Cc: stable@vger.kernel.org      # 5.19
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index ba557328710a..4b0a2d4bb61e 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1535,6 +1535,11 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
 	}
 }
 
+static void of_dp_aux_depopulate_bus_void(void *data)
+{
+	of_dp_aux_depopulate_bus(data);
+}
+
 static int dp_display_get_next_bridge(struct msm_dp *dp)
 {
 	int rc;
@@ -1559,10 +1564,16 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
 		 * panel driver is probed asynchronously but is the best we
 		 * can do without a bigger driver reorganization.
 		 */
-		rc = devm_of_dp_aux_populate_ep_devices(dp_priv->aux);
+		rc = of_dp_aux_populate_bus(dp_priv->aux, NULL);
 		of_node_put(aux_bus);
 		if (rc)
 			goto error;
+
+		rc = devm_add_action_or_reset(dp->drm_dev->dev,
+						of_dp_aux_depopulate_bus_void,
+						dp_priv->aux);
+		if (rc)
+			goto error;
 	} else if (dp->is_edp) {
 		DRM_ERROR("eDP aux_bus not found\n");
 		return -ENODEV;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This can lead resource leaks or failure to bind the aggregate device
when binding is later retried and a second attempt to allocate the
resources is made.

For the DP aux-bus, an attempt to populate the bus a second time will
simply fail ("DP AUX EP device already populated").

Fix this by tying the lifetime of the EP device to the DRM device rather
than DP controller platform device.

Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
Cc: stable@vger.kernel.org      # 5.19
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index ba557328710a..4b0a2d4bb61e 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1535,6 +1535,11 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
 	}
 }
 
+static void of_dp_aux_depopulate_bus_void(void *data)
+{
+	of_dp_aux_depopulate_bus(data);
+}
+
 static int dp_display_get_next_bridge(struct msm_dp *dp)
 {
 	int rc;
@@ -1559,10 +1564,16 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
 		 * panel driver is probed asynchronously but is the best we
 		 * can do without a bigger driver reorganization.
 		 */
-		rc = devm_of_dp_aux_populate_ep_devices(dp_priv->aux);
+		rc = of_dp_aux_populate_bus(dp_priv->aux, NULL);
 		of_node_put(aux_bus);
 		if (rc)
 			goto error;
+
+		rc = devm_add_action_or_reset(dp->drm_dev->dev,
+						of_dp_aux_depopulate_bus_void,
+						dp_priv->aux);
+		if (rc)
+			goto error;
 	} else if (dp->is_edp) {
 		DRM_ERROR("eDP aux_bus not found\n");
 		return -ENODEV;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 07/10] drm/msm/dp: fix bridge lifetime
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This can lead resource leaks or failure to bind the aggregate device
when binding is later retried and a second attempt to allocate the
resources is made.

For the DP bridges, previously allocated bridges will leak on probe
deferral.

Fix this by amending the DP parser interface and tying the lifetime of
the bridge device to the DRM device rather than DP platform device.

Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
Cc: stable@vger.kernel.org      # 5.19
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
 drivers/gpu/drm/msm/dp/dp_parser.c  | 6 +++---
 drivers/gpu/drm/msm/dp/dp_parser.h  | 5 +++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 4b0a2d4bb61e..808a516e84c5 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1586,7 +1586,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
 	 * For DisplayPort interfaces external bridges are optional, so
 	 * silently ignore an error if one is not present (-ENODEV).
 	 */
-	rc = dp_parser_find_next_bridge(dp_priv->parser);
+	rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser);
 	if (!dp->is_edp && rc == -ENODEV)
 		return 0;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index dd732215d55b..dcbe893d66d7 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -240,12 +240,12 @@ static int dp_parser_clock(struct dp_parser *parser)
 	return 0;
 }
 
-int dp_parser_find_next_bridge(struct dp_parser *parser)
+int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser)
 {
-	struct device *dev = &parser->pdev->dev;
+	struct platform_device *pdev = parser->pdev;
 	struct drm_bridge *bridge;
 
-	bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	bridge = devm_drm_of_get_bridge(dev, pdev->dev.of_node, 1, 0);
 	if (IS_ERR(bridge))
 		return PTR_ERR(bridge);
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 866c1a82bf1a..d30ab773db46 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -138,8 +138,9 @@ struct dp_parser {
 struct dp_parser *dp_parser_get(struct platform_device *pdev);
 
 /**
- * dp_parser_find_next_bridge() - find an additional bridge to DP
+ * devm_dp_parser_find_next_bridge() - find an additional bridge to DP
  *
+ * @dev: device to tie bridge lifetime to
  * @parser: dp_parser data from client
  *
  * This function is used to find any additional bridge attached to
@@ -147,6 +148,6 @@ struct dp_parser *dp_parser_get(struct platform_device *pdev);
  *
  * Return: 0 if able to get the bridge, otherwise negative errno for failure.
  */
-int dp_parser_find_next_bridge(struct dp_parser *parser);
+int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser);
 
 #endif
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 07/10] drm/msm/dp: fix bridge lifetime
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This can lead resource leaks or failure to bind the aggregate device
when binding is later retried and a second attempt to allocate the
resources is made.

For the DP bridges, previously allocated bridges will leak on probe
deferral.

Fix this by amending the DP parser interface and tying the lifetime of
the bridge device to the DRM device rather than DP platform device.

Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
Cc: stable@vger.kernel.org      # 5.19
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
 drivers/gpu/drm/msm/dp/dp_parser.c  | 6 +++---
 drivers/gpu/drm/msm/dp/dp_parser.h  | 5 +++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 4b0a2d4bb61e..808a516e84c5 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1586,7 +1586,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
 	 * For DisplayPort interfaces external bridges are optional, so
 	 * silently ignore an error if one is not present (-ENODEV).
 	 */
-	rc = dp_parser_find_next_bridge(dp_priv->parser);
+	rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser);
 	if (!dp->is_edp && rc == -ENODEV)
 		return 0;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index dd732215d55b..dcbe893d66d7 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -240,12 +240,12 @@ static int dp_parser_clock(struct dp_parser *parser)
 	return 0;
 }
 
-int dp_parser_find_next_bridge(struct dp_parser *parser)
+int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser)
 {
-	struct device *dev = &parser->pdev->dev;
+	struct platform_device *pdev = parser->pdev;
 	struct drm_bridge *bridge;
 
-	bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	bridge = devm_drm_of_get_bridge(dev, pdev->dev.of_node, 1, 0);
 	if (IS_ERR(bridge))
 		return PTR_ERR(bridge);
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 866c1a82bf1a..d30ab773db46 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -138,8 +138,9 @@ struct dp_parser {
 struct dp_parser *dp_parser_get(struct platform_device *pdev);
 
 /**
- * dp_parser_find_next_bridge() - find an additional bridge to DP
+ * devm_dp_parser_find_next_bridge() - find an additional bridge to DP
  *
+ * @dev: device to tie bridge lifetime to
  * @parser: dp_parser data from client
  *
  * This function is used to find any additional bridge attached to
@@ -147,6 +148,6 @@ struct dp_parser *dp_parser_get(struct platform_device *pdev);
  *
  * Return: 0 if able to get the bridge, otherwise negative errno for failure.
  */
-int dp_parser_find_next_bridge(struct dp_parser *parser);
+int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser);
 
 #endif
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold, stable

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This is specifically true for the HDMI IRQ, which will otherwise remain
requested so that the next bind attempt fails when requesting the IRQ a
second time.

Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM
device so that it is released when bind fails.

Fixes: 067fef372c73 ("drm/msm/hdmi: refactor bind/init")
Cc: stable@vger.kernel.org      # 3.19
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index a0ed6aa8e4e1..f28fb21e3891 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -344,7 +344,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 		goto fail;
 	}
 
-	ret = devm_request_irq(&pdev->dev, hdmi->irq,
+	ret = devm_request_irq(dev->dev, hdmi->irq,
 			msm_hdmi_irq, IRQF_TRIGGER_HIGH,
 			"hdmi_isr", hdmi);
 	if (ret < 0) {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.

This is specifically true for the HDMI IRQ, which will otherwise remain
requested so that the next bind attempt fails when requesting the IRQ a
second time.

Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM
device so that it is released when bind fails.

Fixes: 067fef372c73 ("drm/msm/hdmi: refactor bind/init")
Cc: stable@vger.kernel.org      # 3.19
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index a0ed6aa8e4e1..f28fb21e3891 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -344,7 +344,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 		goto fail;
 	}
 
-	ret = devm_request_irq(&pdev->dev, hdmi->irq,
+	ret = devm_request_irq(dev->dev, hdmi->irq,
 			msm_hdmi_irq, IRQF_TRIGGER_HIGH,
 			"hdmi_isr", hdmi);
 	if (ret < 0) {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold

Drop the overly defensive modeset sanity checks of function parameters
which have already been checked or used by the callers.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 808a516e84c5..33daec11f813 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
 int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 			struct drm_encoder *encoder)
 {
-	struct msm_drm_private *priv;
+	struct msm_drm_private *priv = dev->dev_private;
 	struct dp_display_private *dp_priv;
 	int ret;
 
-	if (WARN_ON(!encoder) || WARN_ON(!dp_display) || WARN_ON(!dev))
-		return -EINVAL;
-
-	priv = dev->dev_private;
-
 	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
 		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
 		return -ENOSPC;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Drop the overly defensive modeset sanity checks of function parameters
which have already been checked or used by the callers.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 808a516e84c5..33daec11f813 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
 int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
 			struct drm_encoder *encoder)
 {
-	struct msm_drm_private *priv;
+	struct msm_drm_private *priv = dev->dev_private;
 	struct dp_display_private *dp_priv;
 	int ret;
 
-	if (WARN_ON(!encoder) || WARN_ON(!dp_display) || WARN_ON(!dev))
-		return -EINVAL;
-
-	priv = dev->dev_private;
-
 	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
 		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
 		return -ENOSPC;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13  8:53   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, Johan Hovold

Drop the overly defensive modeset sanity checks of function parameters
which have already been checked or used by the callers.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dsi/dsi.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 8a95c744972a..31fdee2052be 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)
 int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
 			 struct drm_encoder *encoder)
 {
-	struct msm_drm_private *priv;
+	struct msm_drm_private *priv = dev->dev_private;
 	int ret;
 
-	if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
-		return -EINVAL;
-
-	priv = dev->dev_private;
-
 	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
 		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
 		return -ENOSPC;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
@ 2022-09-13  8:53   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-13  8:53 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Johan Hovold, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

Drop the overly defensive modeset sanity checks of function parameters
which have already been checked or used by the callers.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpu/drm/msm/dsi/dsi.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 8a95c744972a..31fdee2052be 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)
 int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
 			 struct drm_encoder *encoder)
 {
-	struct msm_drm_private *priv;
+	struct msm_drm_private *priv = dev->dev_private;
 	int ret;
 
-	if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
-		return -EINVAL;
-
-	priv = dev->dev_private;
-
 	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
 		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
 		return -ENOSPC;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-13 12:36     ` Doug Anderson
  -1 siblings, 0 replies; 87+ messages in thread
From: Doug Anderson @ 2022-09-13 12:36 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Daniel Vetter, Sean Paul, Stephen Boyd,
	Bjorn Andersson, Manivannan Sadhasivam, Kuogee Hsieh,
	Steev Klimaszewski, dri-devel, linux-arm-msm, freedreno, LKML,
	# 4.0+

Hi,

On Tue, Sep 13, 2022 at 9:58 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This can lead resource leaks or failure to bind the aggregate device
> when binding is later retried and a second attempt to allocate the
> resources is made.
>
> For the DP aux-bus, an attempt to populate the bus a second time will
> simply fail ("DP AUX EP device already populated").
>
> Fix this by tying the lifetime of the EP device to the DRM device rather
> than DP controller platform device.
>
> Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
> Cc: stable@vger.kernel.org      # 5.19
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

This seems fine to me as a short term fix until we get the DP AUX
populating moved to probe.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime
@ 2022-09-13 12:36     ` Doug Anderson
  0 siblings, 0 replies; 87+ messages in thread
From: Doug Anderson @ 2022-09-13 12:36 UTC (permalink / raw)
  To: Johan Hovold
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, LKML, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, Abhinav Kumar,
	Robert Foss, Stephen Boyd, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Dmitry Baryshkov, # 4.0+,
	Kuogee Hsieh, Sean Paul, Steev Klimaszewski

Hi,

On Tue, Sep 13, 2022 at 9:58 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This can lead resource leaks or failure to bind the aggregate device
> when binding is later retried and a second attempt to allocate the
> resources is made.
>
> For the DP aux-bus, an attempt to populate the bus a second time will
> simply fail ("DP AUX EP device already populated").
>
> Fix this by tying the lifetime of the EP device to the DRM device rather
> than DP controller platform device.
>
> Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
> Cc: stable@vger.kernel.org      # 5.19
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

This seems fine to me as a short term fix until we get the DP AUX
populating moved to probe.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-13 20:23   ` Steev Klimaszewski
  -1 siblings, 0 replies; 87+ messages in thread
From: Steev Klimaszewski @ 2022-09-13 20:23 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, dri-devel, linux-arm-msm, freedreno, linux-kernel

Hi Johan,

On 9/13/22 3:53 AM, Johan Hovold wrote:
> The MSM DRM driver is currently broken in multiple ways with respect to
> probe deferral. Not only does the driver currently fail to probe again
> after a late deferral, but due to a related use-after-free bug this also
> triggers NULL-pointer dereferences.
>
> These bugs are not new but have become critical with the release of
> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> yet been loaded.
>
> The underlying problem is lifetime issues due to careless use of
> device-managed resources.
>
> Specifically, device-managed resources allocated post component bind
> must be tied to the lifetime of the aggregate DRM device or they will
> not necessarily be released when binding of the aggregate device is
> deferred.
>
> The following call chain and pseudo code serves as an illustration of
> the problem:
>
>   - platform_probe(pdev1)
>     - dp_display_probe()
>       - component_add()
>
>   - platform_probe(pdev2) 				// last component
>     - dp_display_probe()					// d0
>         - component_add()
>           - try_to_bring_up_aggregate_device()
> 	   - devres_open_group(adev->parent)		// d1
>
> 	   - msm_drm_bind()
> 	     - msm_drm_init()
> 	       - component_bind_all()
> 	         - for_each_component()
> 		   - component_bind()
> 		     - devres_open_group(&pdev->dev)	// d2
> 	             - dp_display_bind()
> 		       - devm_kzalloc(&pdev->dev)	// a1, OK
> 		     - devres_close_group(&pdev->dev)	// d3
>
> 	       - dpu_kms_hw_init()
> 	         - for_each_panel()
> 	           - msm_dp_modeset_init()
> 		     - dp_display_request_irq()
> 		       - devm_request_irq(&pdev->dev)	// a2, BUG
> 		     - if (pdev == pdev2 && condition)
> 		       - return -EPROBE_DEFER;
>
> 	      - if (error)
>   	        - component_unbind_all()
> 	          - for_each_component()
> 		    - component_unbind()
> 		      - dp_display_unbind()
> 		      - devres_release_group(&pdev->dev) // d4, only a1 is freed
>
>             - if (error)
> 	     - devres_release_group(adev->parent)	// d5
>
> The device-managed allocation a2 is buggy as its lifetime is tied to the
> component platform device and will not be released when the aggregate
> device bind fails (e.g. due to a probe deferral).
>
> When pdev2 is later probed again, the attempt to allocate the IRQ a
> second time will fail for pdev1 (which is still bound to its platform
> driver).
>
> This series fixes the lifetime issues by tying the lifetime of a2 (and
> similar allocations) to the lifetime of the aggregate device so that a2
> is released at d5.
>
> In some cases, such has for the DP IRQ, the above situation can also be
> avoided by moving the allocation in question to the platform driver
> probe (d0) or component bind (between d2 and d3). But as doing so is not
> a general fix, this can be done later as a cleanup/optimisation.
>
> Johan
>
> Changes in v2
>   - use a custom devres action instead of amending the AUX bus interface
>     (Doug)
>   - split sanity check fixes and cleanups per bridge type (Dmitry)
>   - add another Fixes tag for the missing bridge counter reset (Dmitry)
>
>
> Johan Hovold (10):
>    drm/msm: fix use-after-free on probe deferral
>    drm/msm/dp: fix memory corruption with too many bridges
>    drm/msm/dsi: fix memory corruption with too many bridges
>    drm/msm/hdmi: fix memory corruption with too many bridges
>    drm/msm/dp: fix IRQ lifetime
>    drm/msm/dp: fix aux-bus EP lifetime
>    drm/msm/dp: fix bridge lifetime
>    drm/msm/hdmi: fix IRQ lifetime
>    drm/msm/dp: drop modeset sanity checks
>    drm/msm/dsi: drop modeset sanity checks
>
>   drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
>   drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
>   drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
>   drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
>   drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
>   drivers/gpu/drm/msm/msm_drv.c       |  1 +
>   6 files changed, 37 insertions(+), 17 deletions(-)
>
I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo 
Yoga C630), and both of them show the same issue:

[ 7.449305] platform ae9a000.displayport-controller: Fixing up cyclic 
dependency with ae01000.mdp [ 7.454344] Unable to handle kernel NULL 
pointer dereference at virtual address 0000000000000008 [ 7.454406] Mem 
abort info: [ 7.454423] ESR = 0x0000000096000004 [ 7.454446] EC = 0x25: 
DABT (current EL), IL = 32 bits [ 7.454475] SET = 0, FnV = 0 [ 7.454494] 
EA = 0, S1PTW = 0 [ 7.454512] FSC = 0x04: level 0 translation fault [ 
7.454539] Data abort info: [ 7.454556] ISV = 0, ISS = 0x00000004 [ 
7.454577] CM = 0, WnR = 0 [ 7.454595] user pgtable: 4k pages, 48-bit 
VAs, pgdp=0000000101504000 [ 7.454629] [0000000000000008] 
pgd=0000000000000000, p4d=0000000000000000 [ 7.454669] Internal error: 
Oops: 96000004 [#1] PREEMPT SMP [ 7.454700] Modules linked in: 
i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm 
mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper 
drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo 
phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom 
pwm_bl [ 7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 
5.19.0-rc8-next-20220728 #26 [ 7.454902] Hardware name: LENOVO 
82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021 [ 7.454941] Workqueue: 
events_unbound deferred_probe_work_func [ 7.454982] pstate: 40400005 
(nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 7.455020] pc : 
dp_display_request_irq+0x50/0xdc [msm] [ 7.455145] lr : 
dp_display_request_irq+0x2c/0xdc [msm] [ 7.455265] sp : ffff800008c1bb30 
[ 7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 
0000000000000000 [ 7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 
x24: 000000000000003a [ 7.455368] x23: ffffc9c918811d30 x22: 
ffff2a5806baa998 x21: ffff2a5806ba3410 [ 7.455410] x20: ffff2a5806baa880 
x19: ffff2a5806baa998 x18: ffffffffffffffff [ 7.455451] x17: 
0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff [ 7.455492] 
x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000 [ 
7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : 
ffffc9c918493078 [ 7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 
x6 : ffff2a5806baa880 [ 7.455616] x5 : ffffc9c8ca2de000 x4 : 
0000000000080004 x3 : 0000000000000000 [ 7.455656] x2 : ffffc9c8ca296000 
x1 : 00000000000000a8 x0 : 0000000000000000 [ 7.455698] Call trace: [ 
7.455714] dp_display_request_irq+0x50/0xdc [msm] [ 7.455834] 
dp_display_probe+0xf8/0x4a4 [msm] [ 7.455950] platform_probe+0x6c/0xc4 [ 
7.455976] really_probe+0xbc/0x2d4 [ 7.455999] 
__driver_probe_device+0x78/0xe0 [ 7.456025] 
driver_probe_device+0x3c/0x13c [ 7.456051] 
__device_attach_driver+0xb8/0x120 [ 7.456077] bus_for_each_drv+0x78/0xd0 
[ 7.456105] __device_attach+0x9c/0x1a0 [ 7.456129] 
device_initial_probe+0x18/0x2c [ 7.456154] bus_probe_device+0x9c/0xa4 [ 
7.456178] deferred_probe_work_func+0x88/0xc0 [ 7.456204] 
process_one_work+0x1d4/0x330 [ 7.456231] worker_thread+0x70/0x42c [ 
7.456255] kthread+0x10c/0x110 [ 7.456278] ret_from_fork+0x10/0x20 [ 
7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400) [ 
7.456341] ---[ end trace 0000000000000000 ]---

This is from the sc8180x, sdm850 is the same call stack, just with 
different addresses.

I do have 
https://lore.kernel.org/all/20220712132258.671263-1-dmitry.baryshkov@linaro.org/ 
applied here which makes the 10th patch not apply cleanly.

It fails actually, but I applied it manually here.


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-09-13 20:23   ` Steev Klimaszewski
  0 siblings, 0 replies; 87+ messages in thread
From: Steev Klimaszewski @ 2022-09-13 20:23 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	Kuogee Hsieh, Sean Paul, Laurent Pinchart

Hi Johan,

On 9/13/22 3:53 AM, Johan Hovold wrote:
> The MSM DRM driver is currently broken in multiple ways with respect to
> probe deferral. Not only does the driver currently fail to probe again
> after a late deferral, but due to a related use-after-free bug this also
> triggers NULL-pointer dereferences.
>
> These bugs are not new but have become critical with the release of
> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> yet been loaded.
>
> The underlying problem is lifetime issues due to careless use of
> device-managed resources.
>
> Specifically, device-managed resources allocated post component bind
> must be tied to the lifetime of the aggregate DRM device or they will
> not necessarily be released when binding of the aggregate device is
> deferred.
>
> The following call chain and pseudo code serves as an illustration of
> the problem:
>
>   - platform_probe(pdev1)
>     - dp_display_probe()
>       - component_add()
>
>   - platform_probe(pdev2) 				// last component
>     - dp_display_probe()					// d0
>         - component_add()
>           - try_to_bring_up_aggregate_device()
> 	   - devres_open_group(adev->parent)		// d1
>
> 	   - msm_drm_bind()
> 	     - msm_drm_init()
> 	       - component_bind_all()
> 	         - for_each_component()
> 		   - component_bind()
> 		     - devres_open_group(&pdev->dev)	// d2
> 	             - dp_display_bind()
> 		       - devm_kzalloc(&pdev->dev)	// a1, OK
> 		     - devres_close_group(&pdev->dev)	// d3
>
> 	       - dpu_kms_hw_init()
> 	         - for_each_panel()
> 	           - msm_dp_modeset_init()
> 		     - dp_display_request_irq()
> 		       - devm_request_irq(&pdev->dev)	// a2, BUG
> 		     - if (pdev == pdev2 && condition)
> 		       - return -EPROBE_DEFER;
>
> 	      - if (error)
>   	        - component_unbind_all()
> 	          - for_each_component()
> 		    - component_unbind()
> 		      - dp_display_unbind()
> 		      - devres_release_group(&pdev->dev) // d4, only a1 is freed
>
>             - if (error)
> 	     - devres_release_group(adev->parent)	// d5
>
> The device-managed allocation a2 is buggy as its lifetime is tied to the
> component platform device and will not be released when the aggregate
> device bind fails (e.g. due to a probe deferral).
>
> When pdev2 is later probed again, the attempt to allocate the IRQ a
> second time will fail for pdev1 (which is still bound to its platform
> driver).
>
> This series fixes the lifetime issues by tying the lifetime of a2 (and
> similar allocations) to the lifetime of the aggregate device so that a2
> is released at d5.
>
> In some cases, such has for the DP IRQ, the above situation can also be
> avoided by moving the allocation in question to the platform driver
> probe (d0) or component bind (between d2 and d3). But as doing so is not
> a general fix, this can be done later as a cleanup/optimisation.
>
> Johan
>
> Changes in v2
>   - use a custom devres action instead of amending the AUX bus interface
>     (Doug)
>   - split sanity check fixes and cleanups per bridge type (Dmitry)
>   - add another Fixes tag for the missing bridge counter reset (Dmitry)
>
>
> Johan Hovold (10):
>    drm/msm: fix use-after-free on probe deferral
>    drm/msm/dp: fix memory corruption with too many bridges
>    drm/msm/dsi: fix memory corruption with too many bridges
>    drm/msm/hdmi: fix memory corruption with too many bridges
>    drm/msm/dp: fix IRQ lifetime
>    drm/msm/dp: fix aux-bus EP lifetime
>    drm/msm/dp: fix bridge lifetime
>    drm/msm/hdmi: fix IRQ lifetime
>    drm/msm/dp: drop modeset sanity checks
>    drm/msm/dsi: drop modeset sanity checks
>
>   drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
>   drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
>   drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
>   drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
>   drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
>   drivers/gpu/drm/msm/msm_drv.c       |  1 +
>   6 files changed, 37 insertions(+), 17 deletions(-)
>
I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo 
Yoga C630), and both of them show the same issue:

[ 7.449305] platform ae9a000.displayport-controller: Fixing up cyclic 
dependency with ae01000.mdp [ 7.454344] Unable to handle kernel NULL 
pointer dereference at virtual address 0000000000000008 [ 7.454406] Mem 
abort info: [ 7.454423] ESR = 0x0000000096000004 [ 7.454446] EC = 0x25: 
DABT (current EL), IL = 32 bits [ 7.454475] SET = 0, FnV = 0 [ 7.454494] 
EA = 0, S1PTW = 0 [ 7.454512] FSC = 0x04: level 0 translation fault [ 
7.454539] Data abort info: [ 7.454556] ISV = 0, ISS = 0x00000004 [ 
7.454577] CM = 0, WnR = 0 [ 7.454595] user pgtable: 4k pages, 48-bit 
VAs, pgdp=0000000101504000 [ 7.454629] [0000000000000008] 
pgd=0000000000000000, p4d=0000000000000000 [ 7.454669] Internal error: 
Oops: 96000004 [#1] PREEMPT SMP [ 7.454700] Modules linked in: 
i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm 
mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper 
drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo 
phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom 
pwm_bl [ 7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 
5.19.0-rc8-next-20220728 #26 [ 7.454902] Hardware name: LENOVO 
82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021 [ 7.454941] Workqueue: 
events_unbound deferred_probe_work_func [ 7.454982] pstate: 40400005 
(nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 7.455020] pc : 
dp_display_request_irq+0x50/0xdc [msm] [ 7.455145] lr : 
dp_display_request_irq+0x2c/0xdc [msm] [ 7.455265] sp : ffff800008c1bb30 
[ 7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 
0000000000000000 [ 7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 
x24: 000000000000003a [ 7.455368] x23: ffffc9c918811d30 x22: 
ffff2a5806baa998 x21: ffff2a5806ba3410 [ 7.455410] x20: ffff2a5806baa880 
x19: ffff2a5806baa998 x18: ffffffffffffffff [ 7.455451] x17: 
0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff [ 7.455492] 
x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000 [ 
7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : 
ffffc9c918493078 [ 7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 
x6 : ffff2a5806baa880 [ 7.455616] x5 : ffffc9c8ca2de000 x4 : 
0000000000080004 x3 : 0000000000000000 [ 7.455656] x2 : ffffc9c8ca296000 
x1 : 00000000000000a8 x0 : 0000000000000000 [ 7.455698] Call trace: [ 
7.455714] dp_display_request_irq+0x50/0xdc [msm] [ 7.455834] 
dp_display_probe+0xf8/0x4a4 [msm] [ 7.455950] platform_probe+0x6c/0xc4 [ 
7.455976] really_probe+0xbc/0x2d4 [ 7.455999] 
__driver_probe_device+0x78/0xe0 [ 7.456025] 
driver_probe_device+0x3c/0x13c [ 7.456051] 
__device_attach_driver+0xb8/0x120 [ 7.456077] bus_for_each_drv+0x78/0xd0 
[ 7.456105] __device_attach+0x9c/0x1a0 [ 7.456129] 
device_initial_probe+0x18/0x2c [ 7.456154] bus_probe_device+0x9c/0xa4 [ 
7.456178] deferred_probe_work_func+0x88/0xc0 [ 7.456204] 
process_one_work+0x1d4/0x330 [ 7.456231] worker_thread+0x70/0x42c [ 
7.456255] kthread+0x10c/0x110 [ 7.456278] ret_from_fork+0x10/0x20 [ 
7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400) [ 
7.456341] ---[ end trace 0000000000000000 ]---

This is from the sc8180x, sdm850 is the same call stack, just with 
different addresses.

I do have 
https://lore.kernel.org/all/20220712132258.671263-1-dmitry.baryshkov@linaro.org/ 
applied here which makes the 10th patch not apply cleanly.

It fails actually, but I applied it manually here.


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-09-13 20:23   ` Steev Klimaszewski
@ 2022-09-14  6:01     ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-14  6:01 UTC (permalink / raw)
  To: Steev Klimaszewski
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Daniel Vetter,
	Sean Paul, Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, dri-devel, linux-arm-msm, freedreno, linux-kernel

On Tue, Sep 13, 2022 at 03:23:10PM -0500, Steev Klimaszewski wrote:
> Hi Johan,
> 
> On 9/13/22 3:53 AM, Johan Hovold wrote:
> > The MSM DRM driver is currently broken in multiple ways with respect to
> > probe deferral. Not only does the driver currently fail to probe again
> > after a late deferral, but due to a related use-after-free bug this also
> > triggers NULL-pointer dereferences.

> > In some cases, such has for the DP IRQ, the above situation can also be
> > avoided by moving the allocation in question to the platform driver
> > probe (d0) or component bind (between d2 and d3). But as doing so is not
> > a general fix, this can be done later as a cleanup/optimisation.

> I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo 
> Yoga C630), and both of them show the same issue:

[ Copied the below from IRC instead as the formatting in your mail was
off. ]

> [    7.449305] platform ae9a000.displayport-controller: Fixing up cyclic dependency with ae01000.mdp
> [    7.454344] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
> [    7.454406] Mem abort info:
> [    7.454423]   ESR = 0x0000000096000004
> [    7.454446]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    7.454475]   SET = 0, FnV = 0
> [    7.454494]   EA = 0, S1PTW = 0
> [    7.454512]   FSC = 0x04: level 0 translation fault
> [    7.454539] Data abort info:
> [    7.454556]   ISV = 0, ISS = 0x00000004
> [    7.454577]   CM = 0, WnR = 0
> [    7.454595] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101504000
> [    7.454629] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> [    7.454669] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [    7.454700] Modules linked in: i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom pwm_bl
> [    7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 5.19.0-rc8-next-20220728 #26
> [    7.454902] Hardware name: LENOVO 82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021
> [    7.454941] Workqueue: events_unbound deferred_probe_work_func
> [    7.454982] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    7.455020] pc : dp_display_request_irq+0x50/0xdc [msm]
> [    7.455145] lr : dp_display_request_irq+0x2c/0xdc [msm]
> [    7.455265] sp : ffff800008c1bb30
> [    7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 0000000000000000
> [    7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 x24: 000000000000003a
> [    7.455368] x23: ffffc9c918811d30 x22: ffff2a5806baa998 x21: ffff2a5806ba3410
> [    7.455410] x20: ffff2a5806baa880 x19: ffff2a5806baa998 x18: ffffffffffffffff
> [    7.455451] x17: 0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff
> [    7.455492] x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000
> [    7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : ffffc9c918493078
> [    7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 x6 : ffff2a5806baa880
> [    7.455616] x5 : ffffc9c8ca2de000 x4 : 0000000000080004 x3 : 0000000000000000
> [    7.455656] x2 : ffffc9c8ca296000 x1 : 00000000000000a8 x0 : 0000000000000000
> [    7.455698] Call trace:
> [    7.455714]  dp_display_request_irq+0x50/0xdc [msm]
> [    7.455834]  dp_display_probe+0xf8/0x4a4 [msm]
> [    7.455950]  platform_probe+0x6c/0xc4
> [    7.455976]  really_probe+0xbc/0x2d4
> [    7.455999]  __driver_probe_device+0x78/0xe0
> [    7.456025]  driver_probe_device+0x3c/0x13c
> [    7.456051]  __device_attach_driver+0xb8/0x120
> [    7.456077]  bus_for_each_drv+0x78/0xd0
> [    7.456105]  __device_attach+0x9c/0x1a0
> [    7.456129]  device_initial_probe+0x18/0x2c
> [    7.456154]  bus_probe_device+0x9c/0xa4
> [    7.456178]  deferred_probe_work_func+0x88/0xc0
> [    7.456204]  process_one_work+0x1d4/0x330
> [    7.456231]  worker_thread+0x70/0x42c
> [    7.456255]  kthread+0x10c/0x110
> [    7.456278]  ret_from_fork+0x10/0x20
> [    7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400)
> [    7.456341] ---[ end trace 0000000000000000 ]---

> This is from the sc8180x, sdm850 is the same call stack, just with 
> different addresses.
> 
> I do have 
> https://lore.kernel.org/all/20220712132258.671263-1-dmitry.baryshkov@linaro.org/ 
> applied here which makes the 10th patch not apply cleanly.

Yeah, that is expected. You need to drop Dmitry's series first. Once you
verified that this series works, you can add it back if you want but you
then need to restore the device pointer used when allocating the irq in
dp_display_request_irq():

-       rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
+       rc = devm_request_irq(&dp->pdev->dev, dp->irq,
 
> It fails actually, but I applied it manually here.

Please drop that series and give this one another spin.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-09-14  6:01     ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-14  6:01 UTC (permalink / raw)
  To: Steev Klimaszewski
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, Kuogee Hsieh, Bjorn Andersson, freedreno,
	Douglas Anderson, Robert Foss, Abhinav Kumar, Laurent Pinchart,
	Andrzej Hajda, Manivannan Sadhasivam, Dmitry Baryshkov,
	linux-arm-msm, Stephen Boyd, Sean Paul, Johan Hovold

On Tue, Sep 13, 2022 at 03:23:10PM -0500, Steev Klimaszewski wrote:
> Hi Johan,
> 
> On 9/13/22 3:53 AM, Johan Hovold wrote:
> > The MSM DRM driver is currently broken in multiple ways with respect to
> > probe deferral. Not only does the driver currently fail to probe again
> > after a late deferral, but due to a related use-after-free bug this also
> > triggers NULL-pointer dereferences.

> > In some cases, such has for the DP IRQ, the above situation can also be
> > avoided by moving the allocation in question to the platform driver
> > probe (d0) or component bind (between d2 and d3). But as doing so is not
> > a general fix, this can be done later as a cleanup/optimisation.

> I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo 
> Yoga C630), and both of them show the same issue:

[ Copied the below from IRC instead as the formatting in your mail was
off. ]

> [    7.449305] platform ae9a000.displayport-controller: Fixing up cyclic dependency with ae01000.mdp
> [    7.454344] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
> [    7.454406] Mem abort info:
> [    7.454423]   ESR = 0x0000000096000004
> [    7.454446]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    7.454475]   SET = 0, FnV = 0
> [    7.454494]   EA = 0, S1PTW = 0
> [    7.454512]   FSC = 0x04: level 0 translation fault
> [    7.454539] Data abort info:
> [    7.454556]   ISV = 0, ISS = 0x00000004
> [    7.454577]   CM = 0, WnR = 0
> [    7.454595] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101504000
> [    7.454629] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> [    7.454669] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [    7.454700] Modules linked in: i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom pwm_bl
> [    7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 5.19.0-rc8-next-20220728 #26
> [    7.454902] Hardware name: LENOVO 82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021
> [    7.454941] Workqueue: events_unbound deferred_probe_work_func
> [    7.454982] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    7.455020] pc : dp_display_request_irq+0x50/0xdc [msm]
> [    7.455145] lr : dp_display_request_irq+0x2c/0xdc [msm]
> [    7.455265] sp : ffff800008c1bb30
> [    7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 0000000000000000
> [    7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 x24: 000000000000003a
> [    7.455368] x23: ffffc9c918811d30 x22: ffff2a5806baa998 x21: ffff2a5806ba3410
> [    7.455410] x20: ffff2a5806baa880 x19: ffff2a5806baa998 x18: ffffffffffffffff
> [    7.455451] x17: 0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff
> [    7.455492] x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000
> [    7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : ffffc9c918493078
> [    7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 x6 : ffff2a5806baa880
> [    7.455616] x5 : ffffc9c8ca2de000 x4 : 0000000000080004 x3 : 0000000000000000
> [    7.455656] x2 : ffffc9c8ca296000 x1 : 00000000000000a8 x0 : 0000000000000000
> [    7.455698] Call trace:
> [    7.455714]  dp_display_request_irq+0x50/0xdc [msm]
> [    7.455834]  dp_display_probe+0xf8/0x4a4 [msm]
> [    7.455950]  platform_probe+0x6c/0xc4
> [    7.455976]  really_probe+0xbc/0x2d4
> [    7.455999]  __driver_probe_device+0x78/0xe0
> [    7.456025]  driver_probe_device+0x3c/0x13c
> [    7.456051]  __device_attach_driver+0xb8/0x120
> [    7.456077]  bus_for_each_drv+0x78/0xd0
> [    7.456105]  __device_attach+0x9c/0x1a0
> [    7.456129]  device_initial_probe+0x18/0x2c
> [    7.456154]  bus_probe_device+0x9c/0xa4
> [    7.456178]  deferred_probe_work_func+0x88/0xc0
> [    7.456204]  process_one_work+0x1d4/0x330
> [    7.456231]  worker_thread+0x70/0x42c
> [    7.456255]  kthread+0x10c/0x110
> [    7.456278]  ret_from_fork+0x10/0x20
> [    7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400)
> [    7.456341] ---[ end trace 0000000000000000 ]---

> This is from the sc8180x, sdm850 is the same call stack, just with 
> different addresses.
> 
> I do have 
> https://lore.kernel.org/all/20220712132258.671263-1-dmitry.baryshkov@linaro.org/ 
> applied here which makes the 10th patch not apply cleanly.

Yeah, that is expected. You need to drop Dmitry's series first. Once you
verified that this series works, you can add it back if you want but you
then need to restore the device pointer used when allocating the irq in
dp_display_request_irq():

-       rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
+       rc = devm_request_irq(&dp->pdev->dev, dp->irq,
 
> It fails actually, but I applied it manually here.

Please drop that series and give this one another spin.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-09-14  6:01     ` Johan Hovold
@ 2022-09-16 16:43       ` Steev Klimaszewski
  -1 siblings, 0 replies; 87+ messages in thread
From: Steev Klimaszewski @ 2022-09-16 16:43 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Daniel Vetter,
	Sean Paul, Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, dri-devel, linux-arm-msm, freedreno, linux-kernel

Hi Johan,

On Wed, Sep 14, 2022 at 1:01 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Tue, Sep 13, 2022 at 03:23:10PM -0500, Steev Klimaszewski wrote:
> > Hi Johan,
> >
> > On 9/13/22 3:53 AM, Johan Hovold wrote:
> > > The MSM DRM driver is currently broken in multiple ways with respect to
> > > probe deferral. Not only does the driver currently fail to probe again
> > > after a late deferral, but due to a related use-after-free bug this also
> > > triggers NULL-pointer dereferences.
>
> > > In some cases, such has for the DP IRQ, the above situation can also be
> > > avoided by moving the allocation in question to the platform driver
> > > probe (d0) or component bind (between d2 and d3). But as doing so is not
> > > a general fix, this can be done later as a cleanup/optimisation.
>
> > I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo
> > Yoga C630), and both of them show the same issue:
>
> [ Copied the below from IRC instead as the formatting in your mail was
> off. ]
>
> > [    7.449305] platform ae9a000.displayport-controller: Fixing up cyclic dependency with ae01000.mdp
> > [    7.454344] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
> > [    7.454406] Mem abort info:
> > [    7.454423]   ESR = 0x0000000096000004
> > [    7.454446]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [    7.454475]   SET = 0, FnV = 0
> > [    7.454494]   EA = 0, S1PTW = 0
> > [    7.454512]   FSC = 0x04: level 0 translation fault
> > [    7.454539] Data abort info:
> > [    7.454556]   ISV = 0, ISS = 0x00000004
> > [    7.454577]   CM = 0, WnR = 0
> > [    7.454595] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101504000
> > [    7.454629] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> > [    7.454669] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> > [    7.454700] Modules linked in: i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom pwm_bl
> > [    7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 5.19.0-rc8-next-20220728 #26
> > [    7.454902] Hardware name: LENOVO 82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021
> > [    7.454941] Workqueue: events_unbound deferred_probe_work_func
> > [    7.454982] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [    7.455020] pc : dp_display_request_irq+0x50/0xdc [msm]
> > [    7.455145] lr : dp_display_request_irq+0x2c/0xdc [msm]
> > [    7.455265] sp : ffff800008c1bb30
> > [    7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 0000000000000000
> > [    7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 x24: 000000000000003a
> > [    7.455368] x23: ffffc9c918811d30 x22: ffff2a5806baa998 x21: ffff2a5806ba3410
> > [    7.455410] x20: ffff2a5806baa880 x19: ffff2a5806baa998 x18: ffffffffffffffff
> > [    7.455451] x17: 0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff
> > [    7.455492] x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000
> > [    7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : ffffc9c918493078
> > [    7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 x6 : ffff2a5806baa880
> > [    7.455616] x5 : ffffc9c8ca2de000 x4 : 0000000000080004 x3 : 0000000000000000
> > [    7.455656] x2 : ffffc9c8ca296000 x1 : 00000000000000a8 x0 : 0000000000000000
> > [    7.455698] Call trace:
> > [    7.455714]  dp_display_request_irq+0x50/0xdc [msm]
> > [    7.455834]  dp_display_probe+0xf8/0x4a4 [msm]
> > [    7.455950]  platform_probe+0x6c/0xc4
> > [    7.455976]  really_probe+0xbc/0x2d4
> > [    7.455999]  __driver_probe_device+0x78/0xe0
> > [    7.456025]  driver_probe_device+0x3c/0x13c
> > [    7.456051]  __device_attach_driver+0xb8/0x120
> > [    7.456077]  bus_for_each_drv+0x78/0xd0
> > [    7.456105]  __device_attach+0x9c/0x1a0
> > [    7.456129]  device_initial_probe+0x18/0x2c
> > [    7.456154]  bus_probe_device+0x9c/0xa4
> > [    7.456178]  deferred_probe_work_func+0x88/0xc0
> > [    7.456204]  process_one_work+0x1d4/0x330
> > [    7.456231]  worker_thread+0x70/0x42c
> > [    7.456255]  kthread+0x10c/0x110
> > [    7.456278]  ret_from_fork+0x10/0x20
> > [    7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400)
> > [    7.456341] ---[ end trace 0000000000000000 ]---
>
> > This is from the sc8180x, sdm850 is the same call stack, just with
> > different addresses.
> >
> > I do have
> > https://lore.kernel.org/all/20220712132258.671263-1-dmitry.baryshkov@linaro.org/
> > applied here which makes the 10th patch not apply cleanly.
>
> Yeah, that is expected. You need to drop Dmitry's series first. Once you
> verified that this series works, you can add it back if you want but you
> then need to restore the device pointer used when allocating the irq in
> dp_display_request_irq():
>
> -       rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
> +       rc = devm_request_irq(&dp->pdev->dev, dp->irq,
>
> > It fails actually, but I applied it manually here.
>
> Please drop that series and give this one another spin.
>
> Johan

I thought as much but wasn't sure.  Thanks for the clarification.
With Dmitriy's patchset backed out, this series does work as expected.

Tested-by: Steev Klimaszewski <steev@kali.org>

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-09-16 16:43       ` Steev Klimaszewski
  0 siblings, 0 replies; 87+ messages in thread
From: Steev Klimaszewski @ 2022-09-16 16:43 UTC (permalink / raw)
  To: Johan Hovold
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, Kuogee Hsieh, Bjorn Andersson, freedreno,
	Douglas Anderson, Robert Foss, Abhinav Kumar, Laurent Pinchart,
	Andrzej Hajda, Manivannan Sadhasivam, Dmitry Baryshkov,
	linux-arm-msm, Stephen Boyd, Sean Paul, Johan Hovold

Hi Johan,

On Wed, Sep 14, 2022 at 1:01 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Tue, Sep 13, 2022 at 03:23:10PM -0500, Steev Klimaszewski wrote:
> > Hi Johan,
> >
> > On 9/13/22 3:53 AM, Johan Hovold wrote:
> > > The MSM DRM driver is currently broken in multiple ways with respect to
> > > probe deferral. Not only does the driver currently fail to probe again
> > > after a late deferral, but due to a related use-after-free bug this also
> > > triggers NULL-pointer dereferences.
>
> > > In some cases, such has for the DP IRQ, the above situation can also be
> > > avoided by moving the allocation in question to the platform driver
> > > probe (d0) or component bind (between d2 and d3). But as doing so is not
> > > a general fix, this can be done later as a cleanup/optimisation.
>
> > I've tested this on both sc8180x (Lenovo Flex 5G) and sdm850 (Lenovo
> > Yoga C630), and both of them show the same issue:
>
> [ Copied the below from IRC instead as the formatting in your mail was
> off. ]
>
> > [    7.449305] platform ae9a000.displayport-controller: Fixing up cyclic dependency with ae01000.mdp
> > [    7.454344] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
> > [    7.454406] Mem abort info:
> > [    7.454423]   ESR = 0x0000000096000004
> > [    7.454446]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [    7.454475]   SET = 0, FnV = 0
> > [    7.454494]   EA = 0, S1PTW = 0
> > [    7.454512]   FSC = 0x04: level 0 translation fault
> > [    7.454539] Data abort info:
> > [    7.454556]   ISV = 0, ISS = 0x00000004
> > [    7.454577]   CM = 0, WnR = 0
> > [    7.454595] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101504000
> > [    7.454629] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> > [    7.454669] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> > [    7.454700] Modules linked in: i2c_hid_of i2c_hid leds_qcom_lpg led_class_multicolor rtc_pm8xxx msm mdt_loader gpu_sched drm_dp_aux_bus drm_display_helper drm_kms_helper drm phy_qcom_edp llcc_qcom i2c_qcom_geni phy_qcom_qmp_combo phy_qcom_snps_femto_v2 phy_qcom_qmp_ufs phy_qcom_qmp_pcie ufs_qcom pwm_bl
> > [    7.454860] CPU: 2 PID: 76 Comm: kworker/u16:2 Not tainted 5.19.0-rc8-next-20220728 #26
> > [    7.454902] Hardware name: LENOVO 82AK/LNVNB161216, BIOS EACN43WW(V1.15) 09/13/2021
> > [    7.454941] Workqueue: events_unbound deferred_probe_work_func
> > [    7.454982] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [    7.455020] pc : dp_display_request_irq+0x50/0xdc [msm]
> > [    7.455145] lr : dp_display_request_irq+0x2c/0xdc [msm]
> > [    7.455265] sp : ffff800008c1bb30
> > [    7.455285] x29: ffff800008c1bb30 x28: 0000000000000000 x27: 0000000000000000
> > [    7.455327] x26: ffffc9c918420000 x25: ffffc9c9186ec570 x24: 000000000000003a
> > [    7.455368] x23: ffffc9c918811d30 x22: ffff2a5806baa998 x21: ffff2a5806ba3410
> > [    7.455410] x20: ffff2a5806baa880 x19: ffff2a5806baa998 x18: ffffffffffffffff
> > [    7.455451] x17: 0000000000000038 x16: ffffc9c9164eeb24 x15: ffffffffffffffff
> > [    7.455492] x14: ffff2a5806bc3004 x13: ffff2a5806bc3000 x12: 0000000000000000
> > [    7.455533] x11: 0000000000000040 x10: ffffc9c918493080 x9 : ffffc9c918493078
> > [    7.455574] x8 : ffff2a5800681b88 x7 : 0000000000000000 x6 : ffff2a5806baa880
> > [    7.455616] x5 : ffffc9c8ca2de000 x4 : 0000000000080004 x3 : 0000000000000000
> > [    7.455656] x2 : ffffc9c8ca296000 x1 : 00000000000000a8 x0 : 0000000000000000
> > [    7.455698] Call trace:
> > [    7.455714]  dp_display_request_irq+0x50/0xdc [msm]
> > [    7.455834]  dp_display_probe+0xf8/0x4a4 [msm]
> > [    7.455950]  platform_probe+0x6c/0xc4
> > [    7.455976]  really_probe+0xbc/0x2d4
> > [    7.455999]  __driver_probe_device+0x78/0xe0
> > [    7.456025]  driver_probe_device+0x3c/0x13c
> > [    7.456051]  __device_attach_driver+0xb8/0x120
> > [    7.456077]  bus_for_each_drv+0x78/0xd0
> > [    7.456105]  __device_attach+0x9c/0x1a0
> > [    7.456129]  device_initial_probe+0x18/0x2c
> > [    7.456154]  bus_probe_device+0x9c/0xa4
> > [    7.456178]  deferred_probe_work_func+0x88/0xc0
> > [    7.456204]  process_one_work+0x1d4/0x330
> > [    7.456231]  worker_thread+0x70/0x42c
> > [    7.456255]  kthread+0x10c/0x110
> > [    7.456278]  ret_from_fork+0x10/0x20
> > [    7.456306] Code: aa1403e6 f2a00104 f0000225 f0ffffe2 (f9400400)
> > [    7.456341] ---[ end trace 0000000000000000 ]---
>
> > This is from the sc8180x, sdm850 is the same call stack, just with
> > different addresses.
> >
> > I do have
> > https://lore.kernel.org/all/20220712132258.671263-1-dmitry.baryshkov@linaro.org/
> > applied here which makes the 10th patch not apply cleanly.
>
> Yeah, that is expected. You need to drop Dmitry's series first. Once you
> verified that this series works, you can add it back if you want but you
> then need to restore the device pointer used when allocating the irq in
> dp_display_request_irq():
>
> -       rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
> +       rc = devm_request_irq(&dp->pdev->dev, dp->irq,
>
> > It fails actually, but I applied it manually here.
>
> Please drop that series and give this one another spin.
>
> Johan

I thought as much but wasn't sure.  Thanks for the clarification.
With Dmitriy's patchset backed out, this series does work as expected.

Tested-by: Steev Klimaszewski <steev@kali.org>

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-09-13  8:53 ` Johan Hovold
@ 2022-09-20  9:06   ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-20  9:06 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	Kuogee Hsieh, Sean Paul, Steev Klimaszewski, Laurent Pinchart

On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> The MSM DRM driver is currently broken in multiple ways with respect to
> probe deferral. Not only does the driver currently fail to probe again
> after a late deferral, but due to a related use-after-free bug this also
> triggers NULL-pointer dereferences.
> 
> These bugs are not new but have become critical with the release of
> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> yet been loaded.
> 
> The underlying problem is lifetime issues due to careless use of
> device-managed resources.

Any chance of getting this merged for 6.1?

Johan

> Changes in v2
>  - use a custom devres action instead of amending the AUX bus interface
>    (Doug)
>  - split sanity check fixes and cleanups per bridge type (Dmitry)
>  - add another Fixes tag for the missing bridge counter reset (Dmitry)
> 
> 
> Johan Hovold (10):
>   drm/msm: fix use-after-free on probe deferral
>   drm/msm/dp: fix memory corruption with too many bridges
>   drm/msm/dsi: fix memory corruption with too many bridges
>   drm/msm/hdmi: fix memory corruption with too many bridges
>   drm/msm/dp: fix IRQ lifetime
>   drm/msm/dp: fix aux-bus EP lifetime
>   drm/msm/dp: fix bridge lifetime
>   drm/msm/hdmi: fix IRQ lifetime
>   drm/msm/dp: drop modeset sanity checks
>   drm/msm/dsi: drop modeset sanity checks
> 
>  drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
>  drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
>  drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
>  drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
>  drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
>  drivers/gpu/drm/msm/msm_drv.c       |  1 +
>  6 files changed, 37 insertions(+), 17 deletions(-)

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-09-20  9:06   ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-20  9:06 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel

On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> The MSM DRM driver is currently broken in multiple ways with respect to
> probe deferral. Not only does the driver currently fail to probe again
> after a late deferral, but due to a related use-after-free bug this also
> triggers NULL-pointer dereferences.
> 
> These bugs are not new but have become critical with the release of
> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> yet been loaded.
> 
> The underlying problem is lifetime issues due to careless use of
> device-managed resources.

Any chance of getting this merged for 6.1?

Johan

> Changes in v2
>  - use a custom devres action instead of amending the AUX bus interface
>    (Doug)
>  - split sanity check fixes and cleanups per bridge type (Dmitry)
>  - add another Fixes tag for the missing bridge counter reset (Dmitry)
> 
> 
> Johan Hovold (10):
>   drm/msm: fix use-after-free on probe deferral
>   drm/msm/dp: fix memory corruption with too many bridges
>   drm/msm/dsi: fix memory corruption with too many bridges
>   drm/msm/hdmi: fix memory corruption with too many bridges
>   drm/msm/dp: fix IRQ lifetime
>   drm/msm/dp: fix aux-bus EP lifetime
>   drm/msm/dp: fix bridge lifetime
>   drm/msm/hdmi: fix IRQ lifetime
>   drm/msm/dp: drop modeset sanity checks
>   drm/msm/dsi: drop modeset sanity checks
> 
>  drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
>  drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
>  drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
>  drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
>  drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
>  drivers/gpu/drm/msm/msm_drv.c       |  1 +
>  6 files changed, 37 insertions(+), 17 deletions(-)

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:48     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:48 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> The bridge counter was never reset when tearing down the DRM device so
> that stale pointers to deallocated structures would be accessed on the
> next tear down (e.g. after a second late bind deferral).
>
> Given enough bridges and a few probe deferrals this could currently also
> lead to data beyond the bridge array being corrupted.
>
> Fixes: d28ea556267c ("drm/msm: properly add and remove internal bridges")
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org      # 3.12
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

> ---
>   drivers/gpu/drm/msm/msm_drv.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 391d86b54ded..d254fe2507ec 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)
>   
>   	for (i = 0; i < priv->num_bridges; i++)
>   		drm_bridge_remove(priv->bridges[i]);
> +	priv->num_bridges = 0;
>   
>   	pm_runtime_get_sync(dev);
>   	msm_irq_uninstall(ddev);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
@ 2022-09-22 19:48     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:48 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	stable, freedreno, Robert Foss, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> The bridge counter was never reset when tearing down the DRM device so
> that stale pointers to deallocated structures would be accessed on the
> next tear down (e.g. after a second late bind deferral).
>
> Given enough bridges and a few probe deferrals this could currently also
> lead to data beyond the bridge array being corrupted.
>
> Fixes: d28ea556267c ("drm/msm: properly add and remove internal bridges")
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org      # 3.12
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

> ---
>   drivers/gpu/drm/msm/msm_drv.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 391d86b54ded..d254fe2507ec 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)
>   
>   	for (i = 0; i < priv->num_bridges; i++)
>   		drm_bridge_remove(priv->bridges[i]);
> +	priv->num_bridges = 0;
>   
>   	pm_runtime_get_sync(dev);
>   	msm_irq_uninstall(ddev);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:51     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:51 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
>
> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> Cc: stable@vger.kernel.org	# 5.17
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 3e284fed8d30..fbe950edaefe 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1604,6 +1604,12 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   		return -EINVAL;
>   
>   	priv = dev->dev_private;
> +
> +	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
> +		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
> +		return -ENOSPC;
> +	}
> +
>   	dp_display->drm_dev = dev;
>   
>   	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
@ 2022-09-22 19:51     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:51 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	stable, freedreno, Robert Foss, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
>
> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> Cc: stable@vger.kernel.org	# 5.17
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 3e284fed8d30..fbe950edaefe 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1604,6 +1604,12 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   		return -EINVAL;
>   
>   	priv = dev->dev_private;
> +
> +	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
> +		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
> +		return -ENOSPC;
> +	}
> +
>   	dp_display->drm_dev = dev;
>   
>   	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:51     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:51 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, linux-kernel, Jernej Skrabec, Stephen Boyd,
	Robert Foss, Andrzej Hajda, Manivannan Sadhasivam, stable,
	freedreno, Sean Paul, Steev Klimaszewski, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
>
> Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> Cc: stable@vger.kernel.org	# 4.1
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 39bbabb5daf6..8a95c744972a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -218,6 +218,12 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>   		return -EINVAL;
>   
>   	priv = dev->dev_private;
> +
> +	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
> +		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
> +		return -ENOSPC;
> +	}
> +
>   	msm_dsi->dev = dev;
>   
>   	ret = msm_dsi_host_modeset_init(msm_dsi->host, dev);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
@ 2022-09-22 19:51     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:51 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Steev Klimaszewski, dri-devel, linux-arm-msm, freedreno,
	linux-kernel, stable


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
>
> Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> Cc: stable@vger.kernel.org	# 4.1
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 39bbabb5daf6..8a95c744972a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -218,6 +218,12 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>   		return -EINVAL;
>   
>   	priv = dev->dev_private;
> +
> +	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
> +		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
> +		return -ENOSPC;
> +	}
> +
>   	msm_dsi->dev = dev;
>   
>   	ret = msm_dsi_host_modeset_init(msm_dsi->host, dev);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:52     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:52 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	stable, freedreno, Robert Foss, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
>
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org	# 3.12
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 93fe61b86967..a0ed6aa8e4e1 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -300,6 +300,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   	struct platform_device *pdev = hdmi->pdev;
>   	int ret;
>   
> +	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
> +		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
> +		return -ENOSPC;
> +	}
> +
>   	hdmi->dev = dev;
>   	hdmi->encoder = encoder;
>   

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
@ 2022-09-22 19:52     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:52 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
>
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org	# 3.12
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 93fe61b86967..a0ed6aa8e4e1 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -300,6 +300,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   	struct platform_device *pdev = hdmi->pdev;
>   	int ret;
>   
> +	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
> +		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
> +		return -ENOSPC;
> +	}
> +
>   	hdmi->dev = dev;
>   	hdmi->encoder = encoder;
>   

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 05/10] drm/msm/dp: fix IRQ lifetime
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:54     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This is specifically true for the DP IRQ, which will otherwise remain
> requested so that the next bind attempt fails when requesting the IRQ a
> second time.
>
> Since commit c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
> this can happen when the aux-bus panel driver has not yet been loaded so
> that probe is deferred.
>
> Fix this by tying the device-managed lifetime of the DP IRQ to the DRM
> device so that it is released when bind fails.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Cc: stable@vger.kernel.org      # 5.10
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@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 fbe950edaefe..ba557328710a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1258,7 +1258,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
>   		return -EINVAL;
>   	}
>   
> -	rc = devm_request_irq(&dp->pdev->dev, dp->irq,
> +	rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
>   			dp_display_irq_handler,
>   			IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>   	if (rc < 0) {

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 05/10] drm/msm/dp: fix IRQ lifetime
@ 2022-09-22 19:54     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	stable, freedreno, Robert Foss, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This is specifically true for the DP IRQ, which will otherwise remain
> requested so that the next bind attempt fails when requesting the IRQ a
> second time.
>
> Since commit c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
> this can happen when the aux-bus panel driver has not yet been loaded so
> that probe is deferred.
>
> Fix this by tying the device-managed lifetime of the DP IRQ to the DRM
> device so that it is released when bind fails.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Cc: stable@vger.kernel.org      # 5.10
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@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 fbe950edaefe..ba557328710a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1258,7 +1258,7 @@ int dp_display_request_irq(struct msm_dp *dp_display)
>   		return -EINVAL;
>   	}
>   
> -	rc = devm_request_irq(&dp->pdev->dev, dp->irq,
> +	rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
>   			dp_display_irq_handler,
>   			IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
>   	if (rc < 0) {

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:54     ` Dmitry Baryshkov
  -1 siblings, 0 replies; 87+ messages in thread
From: Dmitry Baryshkov @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, stable

On 13/09/2022 11:53, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
> 
> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> Cc: stable@vger.kernel.org	# 5.17
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges
@ 2022-09-22 19:54     ` Dmitry Baryshkov
  0 siblings, 0 replies; 87+ messages in thread
From: Dmitry Baryshkov @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	stable, Kuogee Hsieh, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart

On 13/09/2022 11:53, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
> 
> Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
> Cc: stable@vger.kernel.org	# 5.17
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:54     ` Dmitry Baryshkov
  -1 siblings, 0 replies; 87+ messages in thread
From: Dmitry Baryshkov @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, stable

On 13/09/2022 11:53, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
> 
> Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> Cc: stable@vger.kernel.org	# 4.1
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 03/10] drm/msm/dsi: fix memory corruption with too many bridges
@ 2022-09-22 19:54     ` Dmitry Baryshkov
  0 siblings, 0 replies; 87+ messages in thread
From: Dmitry Baryshkov @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	stable, Kuogee Hsieh, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart

On 13/09/2022 11:53, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
> 
> Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> Cc: stable@vger.kernel.org	# 4.1
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:54     ` Dmitry Baryshkov
  -1 siblings, 0 replies; 87+ messages in thread
From: Dmitry Baryshkov @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel, stable

On 13/09/2022 11:53, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
> 
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org	# 3.12
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 04/10] drm/msm/hdmi: fix memory corruption with too many bridges
@ 2022-09-22 19:54     ` Dmitry Baryshkov
  0 siblings, 0 replies; 87+ messages in thread
From: Dmitry Baryshkov @ 2022-09-22 19:54 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	stable, Kuogee Hsieh, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart

On 13/09/2022 11:53, Johan Hovold wrote:
> Add the missing sanity check on the bridge counter to avoid corrupting
> data beyond the fixed-sized bridge array in case there are ever more
> than eight bridges.
> 
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org	# 3.12
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:55     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:55 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Steev Klimaszewski, dri-devel, linux-arm-msm, freedreno,
	linux-kernel, stable


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This is specifically true for the HDMI IRQ, which will otherwise remain
> requested so that the next bind attempt fails when requesting the IRQ a
> second time.
>
> Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM
> device so that it is released when bind fails.
>
> Fixes: 067fef372c73 ("drm/msm/hdmi: refactor bind/init")
> Cc: stable@vger.kernel.org      # 3.19
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index a0ed6aa8e4e1..f28fb21e3891 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -344,7 +344,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   		goto fail;
>   	}
>   
> -	ret = devm_request_irq(&pdev->dev, hdmi->irq,
> +	ret = devm_request_irq(dev->dev, hdmi->irq,
>   			msm_hdmi_irq, IRQF_TRIGGER_HIGH,
>   			"hdmi_isr", hdmi);
>   	if (ret < 0) {

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
@ 2022-09-22 19:55     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:55 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, linux-kernel, Jernej Skrabec, Stephen Boyd,
	Robert Foss, Andrzej Hajda, Manivannan Sadhasivam, stable,
	freedreno, Sean Paul, Steev Klimaszewski, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This is specifically true for the HDMI IRQ, which will otherwise remain
> requested so that the next bind attempt fails when requesting the IRQ a
> second time.
>
> Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM
> device so that it is released when bind fails.
>
> Fixes: 067fef372c73 ("drm/msm/hdmi: refactor bind/init")
> Cc: stable@vger.kernel.org      # 3.19
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index a0ed6aa8e4e1..f28fb21e3891 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -344,7 +344,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   		goto fail;
>   	}
>   
> -	ret = devm_request_irq(&pdev->dev, hdmi->irq,
> +	ret = devm_request_irq(dev->dev, hdmi->irq,
>   			msm_hdmi_irq, IRQF_TRIGGER_HIGH,
>   			"hdmi_isr", hdmi);
>   	if (ret < 0) {

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 07/10] drm/msm/dp: fix bridge lifetime
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:56     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:56 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This can lead resource leaks or failure to bind the aggregate device
> when binding is later retried and a second attempt to allocate the
> resources is made.
>
> For the DP bridges, previously allocated bridges will leak on probe
> deferral.
>
> Fix this by amending the DP parser interface and tying the lifetime of
> the bridge device to the DRM device rather than DP platform device.
>
> Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
> Cc: stable@vger.kernel.org      # 5.19
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
>   drivers/gpu/drm/msm/dp/dp_parser.c  | 6 +++---
>   drivers/gpu/drm/msm/dp/dp_parser.h  | 5 +++--
>   3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 4b0a2d4bb61e..808a516e84c5 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1586,7 +1586,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   	 * For DisplayPort interfaces external bridges are optional, so
>   	 * silently ignore an error if one is not present (-ENODEV).
>   	 */
> -	rc = dp_parser_find_next_bridge(dp_priv->parser);
> +	rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser);
>   	if (!dp->is_edp && rc == -ENODEV)
>   		return 0;
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index dd732215d55b..dcbe893d66d7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -240,12 +240,12 @@ static int dp_parser_clock(struct dp_parser *parser)
>   	return 0;
>   }
>   
> -int dp_parser_find_next_bridge(struct dp_parser *parser)
> +int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser)
>   {
> -	struct device *dev = &parser->pdev->dev;
> +	struct platform_device *pdev = parser->pdev;
>   	struct drm_bridge *bridge;
>   
> -	bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	bridge = devm_drm_of_get_bridge(dev, pdev->dev.of_node, 1, 0);
>   	if (IS_ERR(bridge))
>   		return PTR_ERR(bridge);
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 866c1a82bf1a..d30ab773db46 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -138,8 +138,9 @@ struct dp_parser {
>   struct dp_parser *dp_parser_get(struct platform_device *pdev);
>   
>   /**
> - * dp_parser_find_next_bridge() - find an additional bridge to DP
> + * devm_dp_parser_find_next_bridge() - find an additional bridge to DP
>    *
> + * @dev: device to tie bridge lifetime to
>    * @parser: dp_parser data from client
>    *
>    * This function is used to find any additional bridge attached to
> @@ -147,6 +148,6 @@ struct dp_parser *dp_parser_get(struct platform_device *pdev);
>    *
>    * Return: 0 if able to get the bridge, otherwise negative errno for failure.
>    */
> -int dp_parser_find_next_bridge(struct dp_parser *parser);
> +int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser);
>   
>   #endif

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 07/10] drm/msm/dp: fix bridge lifetime
@ 2022-09-22 19:56     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:56 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	stable, freedreno, Robert Foss, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
>
> This can lead resource leaks or failure to bind the aggregate device
> when binding is later retried and a second attempt to allocate the
> resources is made.
>
> For the DP bridges, previously allocated bridges will leak on probe
> deferral.
>
> Fix this by amending the DP parser interface and tying the lifetime of
> the bridge device to the DRM device rather than DP platform device.
>
> Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
> Cc: stable@vger.kernel.org      # 5.19
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
>   drivers/gpu/drm/msm/dp/dp_parser.c  | 6 +++---
>   drivers/gpu/drm/msm/dp/dp_parser.h  | 5 +++--
>   3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 4b0a2d4bb61e..808a516e84c5 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1586,7 +1586,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   	 * For DisplayPort interfaces external bridges are optional, so
>   	 * silently ignore an error if one is not present (-ENODEV).
>   	 */
> -	rc = dp_parser_find_next_bridge(dp_priv->parser);
> +	rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser);
>   	if (!dp->is_edp && rc == -ENODEV)
>   		return 0;
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index dd732215d55b..dcbe893d66d7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -240,12 +240,12 @@ static int dp_parser_clock(struct dp_parser *parser)
>   	return 0;
>   }
>   
> -int dp_parser_find_next_bridge(struct dp_parser *parser)
> +int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser)
>   {
> -	struct device *dev = &parser->pdev->dev;
> +	struct platform_device *pdev = parser->pdev;
>   	struct drm_bridge *bridge;
>   
> -	bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	bridge = devm_drm_of_get_bridge(dev, pdev->dev.of_node, 1, 0);
>   	if (IS_ERR(bridge))
>   		return PTR_ERR(bridge);
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 866c1a82bf1a..d30ab773db46 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -138,8 +138,9 @@ struct dp_parser {
>   struct dp_parser *dp_parser_get(struct platform_device *pdev);
>   
>   /**
> - * dp_parser_find_next_bridge() - find an additional bridge to DP
> + * devm_dp_parser_find_next_bridge() - find an additional bridge to DP
>    *
> + * @dev: device to tie bridge lifetime to
>    * @parser: dp_parser data from client
>    *
>    * This function is used to find any additional bridge attached to
> @@ -147,6 +148,6 @@ struct dp_parser *dp_parser_get(struct platform_device *pdev);
>    *
>    * Return: 0 if able to get the bridge, otherwise negative errno for failure.
>    */
> -int dp_parser_find_next_bridge(struct dp_parser *parser);
> +int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser);
>   
>   #endif

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:57     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:57 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, linux-kernel, Jernej Skrabec, Stephen Boyd,
	Robert Foss, Andrzej Hajda, Manivannan Sadhasivam, freedreno,
	Sean Paul, Steev Klimaszewski, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 808a516e84c5..33daec11f813 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   			struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	struct dp_display_private *dp_priv;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!dp_display) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-22 19:57     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:57 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Steev Klimaszewski, dri-devel, linux-arm-msm, freedreno,
	linux-kernel


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 808a516e84c5..33daec11f813 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   			struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	struct dp_display_private *dp_priv;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!dp_display) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 19:58     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:58 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	freedreno, Robert Foss, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 8a95c744972a..31fdee2052be 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)
>   int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>   			 struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
@ 2022-09-22 19:58     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 19:58 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 8a95c744972a..31fdee2052be 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)
>   int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>   			 struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-22 20:01     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 20:01 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, stable, Sean Paul, Steev Klimaszewski,
	Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> The bridge counter was never reset when tearing down the DRM device so
> that stale pointers to deallocated structures would be accessed on the
> next tear down (e.g. after a second late bind deferral).
>
> Given enough bridges and a few probe deferrals this could currently also
> lead to data beyond the bridge array being corrupted.
>
> Fixes: d28ea556267c ("drm/msm: properly add and remove internal bridges")
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org      # 3.12
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/msm_drv.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 391d86b54ded..d254fe2507ec 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)
>   
>   	for (i = 0; i < priv->num_bridges; i++)
>   		drm_bridge_remove(priv->bridges[i]);
> +	priv->num_bridges = 0;
>   
>   	pm_runtime_get_sync(dev);
>   	msm_irq_uninstall(ddev);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral
@ 2022-09-22 20:01     ` Kuogee Hsieh
  0 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 20:01 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	stable, freedreno, Robert Foss, Laurent Pinchart


On 9/13/2022 1:53 AM, Johan Hovold wrote:
> The bridge counter was never reset when tearing down the DRM device so
> that stale pointers to deallocated structures would be accessed on the
> next tear down (e.g. after a second late bind deferral).
>
> Given enough bridges and a few probe deferrals this could currently also
> lead to data beyond the bridge array being corrupted.
>
> Fixes: d28ea556267c ("drm/msm: properly add and remove internal bridges")
> Fixes: a3376e3ec81c ("drm/msm: convert to drm_bridge")
> Cc: stable@vger.kernel.org      # 3.12
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/msm_drv.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 391d86b54ded..d254fe2507ec 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -241,6 +241,7 @@ static int msm_drm_uninit(struct device *dev)
>   
>   	for (i = 0; i < priv->num_bridges; i++)
>   		drm_bridge_remove(priv->bridges[i]);
> +	priv->num_bridges = 0;
>   
>   	pm_runtime_get_sync(dev);
>   	msm_irq_uninstall(ddev);

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime
  2022-09-13 12:36     ` Doug Anderson
  (?)
@ 2022-09-22 20:02     ` Kuogee Hsieh
  -1 siblings, 0 replies; 87+ messages in thread
From: Kuogee Hsieh @ 2022-09-22 20:02 UTC (permalink / raw)
  To: Doug Anderson, Johan Hovold
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, LKML, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Abhinav Kumar, Robert Foss,
	Stephen Boyd, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Dmitry Baryshkov, # 4.0+,
	freedreno, Sean Paul, Steev Klimaszewski


On 9/13/2022 5:36 AM, Doug Anderson wrote:
> Hi,
>
> On Tue, Sep 13, 2022 at 9:58 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>> Device-managed resources allocated post component bind must be tied to
>> the lifetime of the aggregate DRM device or they will not necessarily be
>> released when binding of the aggregate device is deferred.
>>
>> This can lead resource leaks or failure to bind the aggregate device
>> when binding is later retried and a second attempt to allocate the
>> resources is made.
>>
>> For the DP aux-bus, an attempt to populate the bus a second time will
>> simply fail ("DP AUX EP device already populated").
>>
>> Fix this by tying the lifetime of the EP device to the DRM device rather
>> than DP controller platform device.
>>
>> Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
>> Cc: stable@vger.kernel.org      # 5.19
>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
> This seems fine to me as a short term fix until we get the DP AUX
> populating moved to probe.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
  2022-09-22 19:55     ` Kuogee Hsieh
@ 2022-09-23  6:17       ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-23  6:17 UTC (permalink / raw)
  To: Kuogee Hsieh
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Abhinav Kumar, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Daniel Vetter,
	Sean Paul, Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Steev Klimaszewski, dri-devel, linux-arm-msm, freedreno,
	linux-kernel, stable

On Thu, Sep 22, 2022 at 12:55:03PM -0700, Kuogee Hsieh wrote:
> 
> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> > Device-managed resources allocated post component bind must be tied to
> > the lifetime of the aggregate DRM device or they will not necessarily be
> > released when binding of the aggregate device is deferred.
> >
> > This is specifically true for the HDMI IRQ, which will otherwise remain
> > requested so that the next bind attempt fails when requesting the IRQ a
> > second time.
> >
> > Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM
> > device so that it is released when bind fails.
> >
> > Fixes: 067fef372c73 ("drm/msm/hdmi: refactor bind/init")
> > Cc: stable@vger.kernel.org      # 3.19
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> > Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> > Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

I believe you meant:

Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

here (i.e. without the '>' quotes). Otherwise the tooling may not pick
these up.

Thanks for reviewing and testing.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime
@ 2022-09-23  6:17       ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-23  6:17 UTC (permalink / raw)
  To: Kuogee Hsieh
  Cc: dri-devel, Douglas Anderson, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Jernej Skrabec, Jonas Karlman,
	linux-arm-msm, Abhinav Kumar, Stephen Boyd, Steev Klimaszewski,
	Sean Paul, Johan Hovold, Neil Armstrong, Bjorn Andersson,
	linux-kernel, Robert Foss, Dmitry Baryshkov, stable, freedreno

On Thu, Sep 22, 2022 at 12:55:03PM -0700, Kuogee Hsieh wrote:
> 
> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> > Device-managed resources allocated post component bind must be tied to
> > the lifetime of the aggregate DRM device or they will not necessarily be
> > released when binding of the aggregate device is deferred.
> >
> > This is specifically true for the HDMI IRQ, which will otherwise remain
> > requested so that the next bind attempt fails when requesting the IRQ a
> > second time.
> >
> > Fix this by tying the device-managed lifetime of the HDMI IRQ to the DRM
> > device so that it is released when bind fails.
> >
> > Fixes: 067fef372c73 ("drm/msm/hdmi: refactor bind/init")
> > Cc: stable@vger.kernel.org      # 3.19
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> > Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> > Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

I believe you meant:

Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>

here (i.e. without the '>' quotes). Otherwise the tooling may not pick
these up.

Thanks for reviewing and testing.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-26 18:17     ` Abhinav Kumar
  -1 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-26 18:17 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	Kuogee Hsieh, Sean Paul, Steev Klimaszewski, Laurent Pinchart



On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

The change LGTM, hence

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

I think we can use below fixes tag so that we can pick up this entire 
series for the fixes cycle.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")

> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 808a516e84c5..33daec11f813 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   			struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	struct dp_display_private *dp_priv;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!dp_display) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-26 18:17     ` Abhinav Kumar
  0 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-26 18:17 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel



On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

The change LGTM, hence

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

I think we can use below fixes tag so that we can pick up this entire 
series for the fixes cycle.

Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")

> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 808a516e84c5..33daec11f813 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1607,15 +1607,10 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   			struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	struct dp_display_private *dp_priv;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!dp_display) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
  2022-09-13  8:53   ` Johan Hovold
@ 2022-09-26 18:21     ` Abhinav Kumar
  -1 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-26 18:21 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark
  Cc: Sean Paul, Neil Armstrong, Andrzej Hajda, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, Steev Klimaszewski, linux-kernel,
	dri-devel, Stephen Boyd, Jernej Skrabec, Manivannan Sadhasivam,
	Kuogee Hsieh, freedreno, Robert Foss, Laurent Pinchart



On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

The change LGTM, hence

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

I think we can use below fixes tag so that we can pick up this entire 
series for the fixes cycle.

Fixes: 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers before
use")

> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 8a95c744972a..31fdee2052be 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)
>   int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>   			 struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
@ 2022-09-26 18:21     ` Abhinav Kumar
  0 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-26 18:21 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark
  Cc: dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart



On 9/13/2022 1:53 AM, Johan Hovold wrote:
> Drop the overly defensive modeset sanity checks of function parameters
> which have already been checked or used by the callers.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

The change LGTM, hence

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

I think we can use below fixes tag so that we can pick up this entire 
series for the fixes cycle.

Fixes: 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers before
use")

> ---
>   drivers/gpu/drm/msm/dsi/dsi.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 8a95c744972a..31fdee2052be 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -211,14 +211,9 @@ void __exit msm_dsi_unregister(void)
>   int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>   			 struct drm_encoder *encoder)
>   {
> -	struct msm_drm_private *priv;
> +	struct msm_drm_private *priv = dev->dev_private;
>   	int ret;
>   
> -	if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
> -		return -EINVAL;
> -
> -	priv = dev->dev_private;
> -
>   	if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>   		DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>   		return -ENOSPC;

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-26 18:17     ` Abhinav Kumar
@ 2022-09-27  7:14       ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-27  7:14 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel

On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> > Drop the overly defensive modeset sanity checks of function parameters
> > which have already been checked or used by the callers.
> > 
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> The change LGTM, hence
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> I think we can use below fixes tag so that we can pick up this entire 
> series for the fixes cycle.
> 
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")

Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
this otherwise as it's not a bug.

You also have to watch out for Sasha and his autosel scripts which will
probably try to backport this to stable if it finds a Fixes tag.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-27  7:14       ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-27  7:14 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson,
	Steev Klimaszewski, freedreno, Douglas Anderson, Robert Foss,
	Stephen Boyd, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Dmitry Baryshkov, Kuogee Hsieh, Sean Paul,
	Johan Hovold

On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> > Drop the overly defensive modeset sanity checks of function parameters
> > which have already been checked or used by the callers.
> > 
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> The change LGTM, hence
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> I think we can use below fixes tag so that we can pick up this entire 
> series for the fixes cycle.
> 
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")

Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
this otherwise as it's not a bug.

You also have to watch out for Sasha and his autosel scripts which will
probably try to backport this to stable if it finds a Fixes tag.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
  2022-09-26 18:21     ` Abhinav Kumar
@ 2022-09-27  7:16       ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-27  7:16 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart

On Mon, Sep 26, 2022 at 11:21:38AM -0700, Abhinav Kumar wrote:
> 
> 
> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> > Drop the overly defensive modeset sanity checks of function parameters
> > which have already been checked or used by the callers.
> > 
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> The change LGTM, hence
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> I think we can use below fixes tag so that we can pick up this entire 
> series for the fixes cycle.
> 
> Fixes: 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers before
> use")

Same here. I wouldn't add a Fixes tag unless required by some DRM
process.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
@ 2022-09-27  7:16       ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-27  7:16 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Sean Paul, Neil Armstrong, Kuogee Hsieh, Andrzej Hajda,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson,
	Steev Klimaszewski, Douglas Anderson, dri-devel, linux-kernel,
	Jernej Skrabec, Manivannan Sadhasivam, Dmitry Baryshkov,
	Robert Foss, Stephen Boyd, freedreno, Johan Hovold,
	Laurent Pinchart

On Mon, Sep 26, 2022 at 11:21:38AM -0700, Abhinav Kumar wrote:
> 
> 
> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> > Drop the overly defensive modeset sanity checks of function parameters
> > which have already been checked or used by the callers.
> > 
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> The change LGTM, hence
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> 
> I think we can use below fixes tag so that we can pick up this entire 
> series for the fixes cycle.
> 
> Fixes: 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers before
> use")

Same here. I wouldn't add a Fixes tag unless required by some DRM
process.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-27  7:14       ` Johan Hovold
@ 2022-09-27 18:42         ` Abhinav Kumar
  -1 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-27 18:42 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel



On 9/27/2022 12:14 AM, Johan Hovold wrote:
> On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
>> On 9/13/2022 1:53 AM, Johan Hovold wrote:
>>> Drop the overly defensive modeset sanity checks of function parameters
>>> which have already been checked or used by the callers.
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>
>> The change LGTM, hence
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> I think we can use below fixes tag so that we can pick up this entire
>> series for the fixes cycle.
>>
>> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> 
> Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
> this otherwise as it's not a bug.
> 
> You also have to watch out for Sasha and his autosel scripts which will
> probably try to backport this to stable if it finds a Fixes tag.
> 
> Johan

Discussed with Rob on IRC, we will apply everything except the last two 
patches of this series in the -fixes and take these two for the next 
kernel rev push.

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-27 18:42         ` Abhinav Kumar
  0 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-27 18:42 UTC (permalink / raw)
  To: Johan Hovold
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson,
	Steev Klimaszewski, freedreno, Douglas Anderson, Robert Foss,
	Stephen Boyd, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Dmitry Baryshkov, Kuogee Hsieh, Sean Paul,
	Johan Hovold



On 9/27/2022 12:14 AM, Johan Hovold wrote:
> On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
>> On 9/13/2022 1:53 AM, Johan Hovold wrote:
>>> Drop the overly defensive modeset sanity checks of function parameters
>>> which have already been checked or used by the callers.
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>
>> The change LGTM, hence
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> I think we can use below fixes tag so that we can pick up this entire
>> series for the fixes cycle.
>>
>> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> 
> Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
> this otherwise as it's not a bug.
> 
> You also have to watch out for Sasha and his autosel scripts which will
> probably try to backport this to stable if it finds a Fixes tag.
> 
> Johan

Discussed with Rob on IRC, we will apply everything except the last two 
patches of this series in the -fixes and take these two for the next 
kernel rev push.

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
  2022-09-27  7:16       ` Johan Hovold
@ 2022-09-27 18:44         ` Abhinav Kumar
  -1 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-27 18:44 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	dri-devel, Neil Armstrong, Daniel Vetter, Jonas Karlman,
	linux-arm-msm, Bjorn Andersson, freedreno, linux-kernel,
	Jernej Skrabec, Stephen Boyd, Robert Foss, Andrzej Hajda,
	Manivannan Sadhasivam, Kuogee Hsieh, Sean Paul,
	Steev Klimaszewski, Laurent Pinchart



On 9/27/2022 12:16 AM, Johan Hovold wrote:
> On Mon, Sep 26, 2022 at 11:21:38AM -0700, Abhinav Kumar wrote:
>>
>>
>> On 9/13/2022 1:53 AM, Johan Hovold wrote:
>>> Drop the overly defensive modeset sanity checks of function parameters
>>> which have already been checked or used by the callers.
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>
>> The change LGTM, hence
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> I think we can use below fixes tag so that we can pick up this entire
>> series for the fixes cycle.
>>
>> Fixes: 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers before
>> use")
> 
> Same here. I wouldn't add a Fixes tag unless required by some DRM
> process.
> 
> Johan

Same response as the prev one, we will apply everything except the last 
two patches of this series in the -fixes and take these two for the next 
kernel rev push.

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [Freedreno] [PATCH v2 10/10] drm/msm/dsi: drop modeset sanity checks
@ 2022-09-27 18:44         ` Abhinav Kumar
  0 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-27 18:44 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Sean Paul, Neil Armstrong, Kuogee Hsieh, Andrzej Hajda,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson,
	Steev Klimaszewski, Douglas Anderson, dri-devel, linux-kernel,
	Jernej Skrabec, Manivannan Sadhasivam, Dmitry Baryshkov,
	Robert Foss, Stephen Boyd, freedreno, Johan Hovold,
	Laurent Pinchart



On 9/27/2022 12:16 AM, Johan Hovold wrote:
> On Mon, Sep 26, 2022 at 11:21:38AM -0700, Abhinav Kumar wrote:
>>
>>
>> On 9/13/2022 1:53 AM, Johan Hovold wrote:
>>> Drop the overly defensive modeset sanity checks of function parameters
>>> which have already been checked or used by the callers.
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>
>> The change LGTM, hence
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> I think we can use below fixes tag so that we can pick up this entire
>> series for the fixes cycle.
>>
>> Fixes: 3f0689e66352 ("drm/msm/dsi: check msm_dsi and dsi pointers before
>> use")
> 
> Same here. I wouldn't add a Fixes tag unless required by some DRM
> process.
> 
> Johan

Same response as the prev one, we will apply everything except the last 
two patches of this series in the -fixes and take these two for the next 
kernel rev push.

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-27 18:42         ` Abhinav Kumar
@ 2022-09-28 12:24           ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-28 12:24 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel

On Tue, Sep 27, 2022 at 11:42:53AM -0700, Abhinav Kumar wrote:
> On 9/27/2022 12:14 AM, Johan Hovold wrote:
> > On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
> >> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> >>> Drop the overly defensive modeset sanity checks of function parameters
> >>> which have already been checked or used by the callers.
> >>>
> >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >>
> >> The change LGTM, hence
> >>
> >> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> >>
> >> I think we can use below fixes tag so that we can pick up this entire
> >> series for the fixes cycle.
> >>
> >> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> > 
> > Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
> > this otherwise as it's not a bug.
> > 
> > You also have to watch out for Sasha and his autosel scripts which will
> > probably try to backport this to stable if it finds a Fixes tag.

> Discussed with Rob on IRC, we will apply everything except the last two 
> patches of this series in the -fixes and take these two for the next 
> kernel rev push.

So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you
mean 6.1 and 6.2?

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-28 12:24           ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-28 12:24 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson,
	Steev Klimaszewski, freedreno, Douglas Anderson, Robert Foss,
	Stephen Boyd, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Dmitry Baryshkov, Kuogee Hsieh, Sean Paul,
	Johan Hovold

On Tue, Sep 27, 2022 at 11:42:53AM -0700, Abhinav Kumar wrote:
> On 9/27/2022 12:14 AM, Johan Hovold wrote:
> > On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
> >> On 9/13/2022 1:53 AM, Johan Hovold wrote:
> >>> Drop the overly defensive modeset sanity checks of function parameters
> >>> which have already been checked or used by the callers.
> >>>
> >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >>
> >> The change LGTM, hence
> >>
> >> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> >>
> >> I think we can use below fixes tag so that we can pick up this entire
> >> series for the fixes cycle.
> >>
> >> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> > 
> > Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
> > this otherwise as it's not a bug.
> > 
> > You also have to watch out for Sasha and his autosel scripts which will
> > probably try to backport this to stable if it finds a Fixes tag.

> Discussed with Rob on IRC, we will apply everything except the last two 
> patches of this series in the -fixes and take these two for the next 
> kernel rev push.

So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you
mean 6.1 and 6.2?

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-28 12:24           ` Johan Hovold
@ 2022-09-28 15:33             ` Abhinav Kumar
  -1 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-28 15:33 UTC (permalink / raw)
  To: Johan Hovold
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson,
	Steev Klimaszewski, freedreno, Douglas Anderson, Robert Foss,
	Stephen Boyd, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Dmitry Baryshkov, Kuogee Hsieh, Sean Paul,
	Johan Hovold



On 9/28/2022 5:24 AM, Johan Hovold wrote:
> On Tue, Sep 27, 2022 at 11:42:53AM -0700, Abhinav Kumar wrote:
>> On 9/27/2022 12:14 AM, Johan Hovold wrote:
>>> On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
>>>> On 9/13/2022 1:53 AM, Johan Hovold wrote:
>>>>> Drop the overly defensive modeset sanity checks of function parameters
>>>>> which have already been checked or used by the callers.
>>>>>
>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>>>
>>>> The change LGTM, hence
>>>>
>>>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>>
>>>> I think we can use below fixes tag so that we can pick up this entire
>>>> series for the fixes cycle.
>>>>
>>>> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
>>>
>>> Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
>>> this otherwise as it's not a bug.
>>>
>>> You also have to watch out for Sasha and his autosel scripts which will
>>> probably try to backport this to stable if it finds a Fixes tag.
> 
>> Discussed with Rob on IRC, we will apply everything except the last two
>> patches of this series in the -fixes and take these two for the next
>> kernel rev push.
> 
> So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you
> mean 6.1 and 6.2?
> 
> Johan

The fixes will go in 6.1 first.

The two follow-on cleanups in 6.2.

Thanks

Abhinav

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-28 15:33             ` Abhinav Kumar
  0 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-09-28 15:33 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel



On 9/28/2022 5:24 AM, Johan Hovold wrote:
> On Tue, Sep 27, 2022 at 11:42:53AM -0700, Abhinav Kumar wrote:
>> On 9/27/2022 12:14 AM, Johan Hovold wrote:
>>> On Mon, Sep 26, 2022 at 11:17:20AM -0700, Abhinav Kumar wrote:
>>>> On 9/13/2022 1:53 AM, Johan Hovold wrote:
>>>>> Drop the overly defensive modeset sanity checks of function parameters
>>>>> which have already been checked or used by the callers.
>>>>>
>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>>>
>>>> The change LGTM, hence
>>>>
>>>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>>
>>>> I think we can use below fixes tag so that we can pick up this entire
>>>> series for the fixes cycle.
>>>>
>>>> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
>>>
>>> Perhaps that's a requirement for drm, but I wouldn't add a Fixes tag for
>>> this otherwise as it's not a bug.
>>>
>>> You also have to watch out for Sasha and his autosel scripts which will
>>> probably try to backport this to stable if it finds a Fixes tag.
> 
>> Discussed with Rob on IRC, we will apply everything except the last two
>> patches of this series in the -fixes and take these two for the next
>> kernel rev push.
> 
> So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you
> mean 6.1 and 6.2?
> 
> Johan

The fixes will go in 6.1 first.

The two follow-on cleanups in 6.2.

Thanks

Abhinav

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
  2022-09-28 15:33             ` Abhinav Kumar
@ 2022-09-28 15:57               ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-28 15:57 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson,
	Steev Klimaszewski, freedreno, Douglas Anderson, Robert Foss,
	Stephen Boyd, Laurent Pinchart, Andrzej Hajda,
	Manivannan Sadhasivam, Dmitry Baryshkov, Kuogee Hsieh, Sean Paul,
	Johan Hovold

On Wed, Sep 28, 2022 at 08:33:52AM -0700, Abhinav Kumar wrote:
> On 9/28/2022 5:24 AM, Johan Hovold wrote:
> > On Tue, Sep 27, 2022 at 11:42:53AM -0700, Abhinav Kumar wrote:

> >> Discussed with Rob on IRC, we will apply everything except the last two
> >> patches of this series in the -fixes and take these two for the next
> >> kernel rev push.
> > 
> > So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you
> > mean 6.1 and 6.2?

> The fixes will go in 6.1 first.
> 
> The two follow-on cleanups in 6.2.

Ok, sounds good. Thanks.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks
@ 2022-09-28 15:57               ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-09-28 15:57 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel

On Wed, Sep 28, 2022 at 08:33:52AM -0700, Abhinav Kumar wrote:
> On 9/28/2022 5:24 AM, Johan Hovold wrote:
> > On Tue, Sep 27, 2022 at 11:42:53AM -0700, Abhinav Kumar wrote:

> >> Discussed with Rob on IRC, we will apply everything except the last two
> >> patches of this series in the -fixes and take these two for the next
> >> kernel rev push.
> > 
> > So the fixes go in 6.0 and the two follow-on cleanups in 6.1? Or did you
> > mean 6.1 and 6.2?

> The fixes will go in 6.1 first.
> 
> The two follow-on cleanups in 6.2.

Ok, sounds good. Thanks.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-09-20  9:06   ` Johan Hovold
@ 2022-10-21  6:27     ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-10-21  6:27 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel

On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> > The MSM DRM driver is currently broken in multiple ways with respect to
> > probe deferral. Not only does the driver currently fail to probe again
> > after a late deferral, but due to a related use-after-free bug this also
> > triggers NULL-pointer dereferences.
> > 
> > These bugs are not new but have become critical with the release of
> > 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> > yet been loaded.
> > 
> > The underlying problem is lifetime issues due to careless use of
> > device-managed resources.
> 
> Any chance of getting this merged for 6.1?

Is anyone picking these up as fixes for 6.1-rc as we discussed?

Johan
 
> > Changes in v2
> >  - use a custom devres action instead of amending the AUX bus interface
> >    (Doug)
> >  - split sanity check fixes and cleanups per bridge type (Dmitry)
> >  - add another Fixes tag for the missing bridge counter reset (Dmitry)
> > 
> > 
> > Johan Hovold (10):
> >   drm/msm: fix use-after-free on probe deferral
> >   drm/msm/dp: fix memory corruption with too many bridges
> >   drm/msm/dsi: fix memory corruption with too many bridges
> >   drm/msm/hdmi: fix memory corruption with too many bridges
> >   drm/msm/dp: fix IRQ lifetime
> >   drm/msm/dp: fix aux-bus EP lifetime
> >   drm/msm/dp: fix bridge lifetime
> >   drm/msm/hdmi: fix IRQ lifetime
> >   drm/msm/dp: drop modeset sanity checks
> >   drm/msm/dsi: drop modeset sanity checks
> > 
> >  drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
> >  drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
> >  drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
> >  drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
> >  drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
> >  drivers/gpu/drm/msm/msm_drv.c       |  1 +
> >  6 files changed, 37 insertions(+), 17 deletions(-)

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-10-21  6:27     ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-10-21  6:27 UTC (permalink / raw)
  To: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	Kuogee Hsieh, Sean Paul, Steev Klimaszewski, Laurent Pinchart

On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> > The MSM DRM driver is currently broken in multiple ways with respect to
> > probe deferral. Not only does the driver currently fail to probe again
> > after a late deferral, but due to a related use-after-free bug this also
> > triggers NULL-pointer dereferences.
> > 
> > These bugs are not new but have become critical with the release of
> > 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> > yet been loaded.
> > 
> > The underlying problem is lifetime issues due to careless use of
> > device-managed resources.
> 
> Any chance of getting this merged for 6.1?

Is anyone picking these up as fixes for 6.1-rc as we discussed?

Johan
 
> > Changes in v2
> >  - use a custom devres action instead of amending the AUX bus interface
> >    (Doug)
> >  - split sanity check fixes and cleanups per bridge type (Dmitry)
> >  - add another Fixes tag for the missing bridge counter reset (Dmitry)
> > 
> > 
> > Johan Hovold (10):
> >   drm/msm: fix use-after-free on probe deferral
> >   drm/msm/dp: fix memory corruption with too many bridges
> >   drm/msm/dsi: fix memory corruption with too many bridges
> >   drm/msm/hdmi: fix memory corruption with too many bridges
> >   drm/msm/dp: fix IRQ lifetime
> >   drm/msm/dp: fix aux-bus EP lifetime
> >   drm/msm/dp: fix bridge lifetime
> >   drm/msm/hdmi: fix IRQ lifetime
> >   drm/msm/dp: drop modeset sanity checks
> >   drm/msm/dsi: drop modeset sanity checks
> > 
> >  drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
> >  drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
> >  drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
> >  drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
> >  drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
> >  drivers/gpu/drm/msm/msm_drv.c       |  1 +
> >  6 files changed, 37 insertions(+), 17 deletions(-)

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-10-21  6:27     ` Johan Hovold
@ 2022-10-21 16:05       ` Abhinav Kumar
  -1 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-10-21 16:05 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Daniel Vetter, Sean Paul,
	Stephen Boyd, Bjorn Andersson, Manivannan Sadhasivam,
	Kuogee Hsieh, Steev Klimaszewski, dri-devel, linux-arm-msm,
	freedreno, linux-kernel

Hi Johan

On 10/20/2022 11:27 PM, Johan Hovold wrote:
> On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
>> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
>>> The MSM DRM driver is currently broken in multiple ways with respect to
>>> probe deferral. Not only does the driver currently fail to probe again
>>> after a late deferral, but due to a related use-after-free bug this also
>>> triggers NULL-pointer dereferences.
>>>
>>> These bugs are not new but have become critical with the release of
>>> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
>>> yet been loaded.
>>>
>>> The underlying problem is lifetime issues due to careless use of
>>> device-managed resources.
>>
>> Any chance of getting this merged for 6.1?
> 
> Is anyone picking these up as fixes for 6.1-rc as we discussed?
> 
> Johan

All of these except the last two ( as discussed ) have landed in the 
-fixes tree

https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb

Thanks

Abhinav

>   
>>> Changes in v2
>>>   - use a custom devres action instead of amending the AUX bus interface
>>>     (Doug)
>>>   - split sanity check fixes and cleanups per bridge type (Dmitry)
>>>   - add another Fixes tag for the missing bridge counter reset (Dmitry)
>>>
>>>
>>> Johan Hovold (10):
>>>    drm/msm: fix use-after-free on probe deferral
>>>    drm/msm/dp: fix memory corruption with too many bridges
>>>    drm/msm/dsi: fix memory corruption with too many bridges
>>>    drm/msm/hdmi: fix memory corruption with too many bridges
>>>    drm/msm/dp: fix IRQ lifetime
>>>    drm/msm/dp: fix aux-bus EP lifetime
>>>    drm/msm/dp: fix bridge lifetime
>>>    drm/msm/hdmi: fix IRQ lifetime
>>>    drm/msm/dp: drop modeset sanity checks
>>>    drm/msm/dsi: drop modeset sanity checks
>>>
>>>   drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
>>>   drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
>>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
>>>   drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
>>>   drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
>>>   drivers/gpu/drm/msm/msm_drv.c       |  1 +
>>>   6 files changed, 37 insertions(+), 17 deletions(-)

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-10-21 16:05       ` Abhinav Kumar
  0 siblings, 0 replies; 87+ messages in thread
From: Abhinav Kumar @ 2022-10-21 16:05 UTC (permalink / raw)
  To: Johan Hovold, Douglas Anderson, Dmitry Baryshkov, Rob Clark
  Cc: dri-devel, Neil Armstrong, Jonas Karlman, linux-arm-msm,
	Bjorn Andersson, freedreno, linux-kernel, Jernej Skrabec,
	Stephen Boyd, Robert Foss, Andrzej Hajda, Manivannan Sadhasivam,
	Kuogee Hsieh, Sean Paul, Steev Klimaszewski, Laurent Pinchart

Hi Johan

On 10/20/2022 11:27 PM, Johan Hovold wrote:
> On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
>> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
>>> The MSM DRM driver is currently broken in multiple ways with respect to
>>> probe deferral. Not only does the driver currently fail to probe again
>>> after a late deferral, but due to a related use-after-free bug this also
>>> triggers NULL-pointer dereferences.
>>>
>>> These bugs are not new but have become critical with the release of
>>> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
>>> yet been loaded.
>>>
>>> The underlying problem is lifetime issues due to careless use of
>>> device-managed resources.
>>
>> Any chance of getting this merged for 6.1?
> 
> Is anyone picking these up as fixes for 6.1-rc as we discussed?
> 
> Johan

All of these except the last two ( as discussed ) have landed in the 
-fixes tree

https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb

Thanks

Abhinav

>   
>>> Changes in v2
>>>   - use a custom devres action instead of amending the AUX bus interface
>>>     (Doug)
>>>   - split sanity check fixes and cleanups per bridge type (Dmitry)
>>>   - add another Fixes tag for the missing bridge counter reset (Dmitry)
>>>
>>>
>>> Johan Hovold (10):
>>>    drm/msm: fix use-after-free on probe deferral
>>>    drm/msm/dp: fix memory corruption with too many bridges
>>>    drm/msm/dsi: fix memory corruption with too many bridges
>>>    drm/msm/hdmi: fix memory corruption with too many bridges
>>>    drm/msm/dp: fix IRQ lifetime
>>>    drm/msm/dp: fix aux-bus EP lifetime
>>>    drm/msm/dp: fix bridge lifetime
>>>    drm/msm/hdmi: fix IRQ lifetime
>>>    drm/msm/dp: drop modeset sanity checks
>>>    drm/msm/dsi: drop modeset sanity checks
>>>
>>>   drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++-------
>>>   drivers/gpu/drm/msm/dp/dp_parser.c  |  6 +++---
>>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  5 +++--
>>>   drivers/gpu/drm/msm/dsi/dsi.c       |  9 +++++----
>>>   drivers/gpu/drm/msm/hdmi/hdmi.c     |  7 ++++++-
>>>   drivers/gpu/drm/msm/msm_drv.c       |  1 +
>>>   6 files changed, 37 insertions(+), 17 deletions(-)

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-10-21 16:05       ` Abhinav Kumar
@ 2022-10-24 11:33         ` Johan Hovold
  -1 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-10-24 11:33 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, linux-arm-msm, Bjorn Andersson, freedreno,
	Douglas Anderson, Robert Foss, Stephen Boyd, Laurent Pinchart,
	Andrzej Hajda, Manivannan Sadhasivam, Dmitry Baryshkov,
	Kuogee Hsieh, Sean Paul, Steev Klimaszewski

On Fri, Oct 21, 2022 at 09:05:52AM -0700, Abhinav Kumar wrote:
> Hi Johan
> 
> On 10/20/2022 11:27 PM, Johan Hovold wrote:
> > On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
> >> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> >>> The MSM DRM driver is currently broken in multiple ways with respect to
> >>> probe deferral. Not only does the driver currently fail to probe again
> >>> after a late deferral, but due to a related use-after-free bug this also
> >>> triggers NULL-pointer dereferences.
> >>>
> >>> These bugs are not new but have become critical with the release of
> >>> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> >>> yet been loaded.
> >>>
> >>> The underlying problem is lifetime issues due to careless use of
> >>> device-managed resources.
> >>
> >> Any chance of getting this merged for 6.1?
> > 
> > Is anyone picking these up as fixes for 6.1-rc as we discussed?
> > 
> > Johan
> 
> All of these except the last two ( as discussed ) have landed in the 
> -fixes tree
> 
> https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb

Ah, perfect, thanks.

When do you expect to send these on so that they end up in linux-next
and eventually Linus's tree?

Note that it looks like something happened with the commit messages when
you applied these. Specifically, the Fixes tags appears to now have a
line break in them and there's also some random whitespace before your
SoB:

	Fixes: c3bf8e21
	
	 ("drm/msm/dp: Add eDP support via aux_bus")
	Cc: stable@vger.kernel.org      # 5.19
	Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
	Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
	Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
	Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
	Patchwork: https://patchwork.freedesktop.org/patch/502667/
	Link: https://lore.kernel.org/r/20220913085320.8577-8-johan+linaro@kernel.org
	
	
	Signed-off-by: Abhinav Kumar's avatarAbhinav Kumar <quic_abhinavk@quicinc.com>

It's possible just the gitlab UI that's messed up, but perhaps you can
double check before they hit linux-next, which should complain about
this otherwise.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-10-24 11:33         ` Johan Hovold
  0 siblings, 0 replies; 87+ messages in thread
From: Johan Hovold @ 2022-10-24 11:33 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Douglas Anderson, Dmitry Baryshkov, Rob Clark, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Daniel Vetter, Sean Paul, Stephen Boyd,
	Bjorn Andersson, Manivannan Sadhasivam, Kuogee Hsieh,
	Steev Klimaszewski, dri-devel, linux-arm-msm, freedreno,
	linux-kernel

On Fri, Oct 21, 2022 at 09:05:52AM -0700, Abhinav Kumar wrote:
> Hi Johan
> 
> On 10/20/2022 11:27 PM, Johan Hovold wrote:
> > On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
> >> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> >>> The MSM DRM driver is currently broken in multiple ways with respect to
> >>> probe deferral. Not only does the driver currently fail to probe again
> >>> after a late deferral, but due to a related use-after-free bug this also
> >>> triggers NULL-pointer dereferences.
> >>>
> >>> These bugs are not new but have become critical with the release of
> >>> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> >>> yet been loaded.
> >>>
> >>> The underlying problem is lifetime issues due to careless use of
> >>> device-managed resources.
> >>
> >> Any chance of getting this merged for 6.1?
> > 
> > Is anyone picking these up as fixes for 6.1-rc as we discussed?
> > 
> > Johan
> 
> All of these except the last two ( as discussed ) have landed in the 
> -fixes tree
> 
> https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb

Ah, perfect, thanks.

When do you expect to send these on so that they end up in linux-next
and eventually Linus's tree?

Note that it looks like something happened with the commit messages when
you applied these. Specifically, the Fixes tags appears to now have a
line break in them and there's also some random whitespace before your
SoB:

	Fixes: c3bf8e21
	
	 ("drm/msm/dp: Add eDP support via aux_bus")
	Cc: stable@vger.kernel.org      # 5.19
	Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
	Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
	Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
	Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
	Patchwork: https://patchwork.freedesktop.org/patch/502667/
	Link: https://lore.kernel.org/r/20220913085320.8577-8-johan+linaro@kernel.org
	
	
	Signed-off-by: Abhinav Kumar's avatarAbhinav Kumar <quic_abhinavk@quicinc.com>

It's possible just the gitlab UI that's messed up, but perhaps you can
double check before they hit linux-next, which should complain about
this otherwise.

Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
  2022-10-24 11:33         ` Johan Hovold
@ 2022-10-24 15:01           ` Rob Clark
  -1 siblings, 0 replies; 87+ messages in thread
From: Rob Clark @ 2022-10-24 15:01 UTC (permalink / raw)
  To: Johan Hovold
  Cc: dri-devel, Neil Armstrong, Jernej Skrabec, linux-kernel,
	Jonas Karlman, Kuogee Hsieh, Bjorn Andersson, freedreno,
	Abhinav Kumar, Robert Foss, Douglas Anderson, Laurent Pinchart,
	Andrzej Hajda, Manivannan Sadhasivam, Dmitry Baryshkov,
	linux-arm-msm, Stephen Boyd, Sean Paul, Steev Klimaszewski

On Mon, Oct 24, 2022 at 4:34 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Oct 21, 2022 at 09:05:52AM -0700, Abhinav Kumar wrote:
> > Hi Johan
> >
> > On 10/20/2022 11:27 PM, Johan Hovold wrote:
> > > On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
> > >> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> > >>> The MSM DRM driver is currently broken in multiple ways with respect to
> > >>> probe deferral. Not only does the driver currently fail to probe again
> > >>> after a late deferral, but due to a related use-after-free bug this also
> > >>> triggers NULL-pointer dereferences.
> > >>>
> > >>> These bugs are not new but have become critical with the release of
> > >>> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> > >>> yet been loaded.
> > >>>
> > >>> The underlying problem is lifetime issues due to careless use of
> > >>> device-managed resources.
> > >>
> > >> Any chance of getting this merged for 6.1?
> > >
> > > Is anyone picking these up as fixes for 6.1-rc as we discussed?
> > >
> > > Johan
> >
> > All of these except the last two ( as discussed ) have landed in the
> > -fixes tree
> >
> > https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb
>
> Ah, perfect, thanks.
>
> When do you expect to send these on so that they end up in linux-next
> and eventually Linus's tree?

I'll send a -fixes PR this week

> Note that it looks like something happened with the commit messages when
> you applied these. Specifically, the Fixes tags appears to now have a
> line break in them and there's also some random whitespace before your
> SoB:
>
>         Fixes: c3bf8e21
>
>          ("drm/msm/dp: Add eDP support via aux_bus")

naw, that is just some problem with gitlab's html generation, the
actual patch is fine ;-)

BR,
-R

>         Cc: stable@vger.kernel.org      # 5.19
>         Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>         Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>         Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>         Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>         Patchwork: https://patchwork.freedesktop.org/patch/502667/
>         Link: https://lore.kernel.org/r/20220913085320.8577-8-johan+linaro@kernel.org
>
>
>         Signed-off-by: Abhinav Kumar's avatarAbhinav Kumar <quic_abhinavk@quicinc.com>
>
> It's possible just the gitlab UI that's messed up, but perhaps you can
> double check before they hit linux-next, which should complain about
> this otherwise.
>
> Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

* Re: [PATCH v2 00/10] drm/msm: probe deferral fixes
@ 2022-10-24 15:01           ` Rob Clark
  0 siblings, 0 replies; 87+ messages in thread
From: Rob Clark @ 2022-10-24 15:01 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Abhinav Kumar, Douglas Anderson, Dmitry Baryshkov, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Daniel Vetter, Sean Paul, Stephen Boyd,
	Bjorn Andersson, Manivannan Sadhasivam, Kuogee Hsieh,
	Steev Klimaszewski, dri-devel, linux-arm-msm, freedreno,
	linux-kernel

On Mon, Oct 24, 2022 at 4:34 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Oct 21, 2022 at 09:05:52AM -0700, Abhinav Kumar wrote:
> > Hi Johan
> >
> > On 10/20/2022 11:27 PM, Johan Hovold wrote:
> > > On Tue, Sep 20, 2022 at 11:06:30AM +0200, Johan Hovold wrote:
> > >> On Tue, Sep 13, 2022 at 10:53:10AM +0200, Johan Hovold wrote:
> > >>> The MSM DRM driver is currently broken in multiple ways with respect to
> > >>> probe deferral. Not only does the driver currently fail to probe again
> > >>> after a late deferral, but due to a related use-after-free bug this also
> > >>> triggers NULL-pointer dereferences.
> > >>>
> > >>> These bugs are not new but have become critical with the release of
> > >>> 5.19 where probe is deferred in case the aux-bus EP panel driver has not
> > >>> yet been loaded.
> > >>>
> > >>> The underlying problem is lifetime issues due to careless use of
> > >>> device-managed resources.
> > >>
> > >> Any chance of getting this merged for 6.1?
> > >
> > > Is anyone picking these up as fixes for 6.1-rc as we discussed?
> > >
> > > Johan
> >
> > All of these except the last two ( as discussed ) have landed in the
> > -fixes tree
> >
> > https://gitlab.freedesktop.org/drm/msm/-/commit/6808abdb33bf90330e70a687d29f038507e06ebb
>
> Ah, perfect, thanks.
>
> When do you expect to send these on so that they end up in linux-next
> and eventually Linus's tree?

I'll send a -fixes PR this week

> Note that it looks like something happened with the commit messages when
> you applied these. Specifically, the Fixes tags appears to now have a
> line break in them and there's also some random whitespace before your
> SoB:
>
>         Fixes: c3bf8e21
>
>          ("drm/msm/dp: Add eDP support via aux_bus")

naw, that is just some problem with gitlab's html generation, the
actual patch is fine ;-)

BR,
-R

>         Cc: stable@vger.kernel.org      # 5.19
>         Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>         Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>         Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>         Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>         Patchwork: https://patchwork.freedesktop.org/patch/502667/
>         Link: https://lore.kernel.org/r/20220913085320.8577-8-johan+linaro@kernel.org
>
>
>         Signed-off-by: Abhinav Kumar's avatarAbhinav Kumar <quic_abhinavk@quicinc.com>
>
> It's possible just the gitlab UI that's messed up, but perhaps you can
> double check before they hit linux-next, which should complain about
> this otherwise.
>
> Johan

^ permalink raw reply	[flat|nested] 87+ messages in thread

end of thread, other threads:[~2022-10-24 16:56 UTC | newest]

Thread overview: 87+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-13  8:53 [PATCH v2 00/10] drm/msm: probe deferral fixes Johan Hovold
2022-09-13  8:53 ` Johan Hovold
2022-09-13  8:53 ` [PATCH v2 01/10] drm/msm: fix use-after-free on probe deferral Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:48   ` [Freedreno] " Kuogee Hsieh
2022-09-22 19:48     ` Kuogee Hsieh
2022-09-22 20:01   ` Kuogee Hsieh
2022-09-22 20:01     ` Kuogee Hsieh
2022-09-13  8:53 ` [PATCH v2 02/10] drm/msm/dp: fix memory corruption with too many bridges Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:51   ` [Freedreno] " Kuogee Hsieh
2022-09-22 19:51     ` Kuogee Hsieh
2022-09-22 19:54   ` Dmitry Baryshkov
2022-09-22 19:54     ` Dmitry Baryshkov
2022-09-13  8:53 ` [PATCH v2 03/10] drm/msm/dsi: " Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:51   ` Kuogee Hsieh
2022-09-22 19:51     ` Kuogee Hsieh
2022-09-22 19:54   ` Dmitry Baryshkov
2022-09-22 19:54     ` Dmitry Baryshkov
2022-09-13  8:53 ` [PATCH v2 04/10] drm/msm/hdmi: " Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:52   ` [Freedreno] " Kuogee Hsieh
2022-09-22 19:52     ` Kuogee Hsieh
2022-09-22 19:54   ` Dmitry Baryshkov
2022-09-22 19:54     ` Dmitry Baryshkov
2022-09-13  8:53 ` [PATCH v2 05/10] drm/msm/dp: fix IRQ lifetime Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:54   ` [Freedreno] " Kuogee Hsieh
2022-09-22 19:54     ` Kuogee Hsieh
2022-09-13  8:53 ` [PATCH v2 06/10] drm/msm/dp: fix aux-bus EP lifetime Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-13 12:36   ` Doug Anderson
2022-09-13 12:36     ` Doug Anderson
2022-09-22 20:02     ` Kuogee Hsieh
2022-09-13  8:53 ` [PATCH v2 07/10] drm/msm/dp: fix bridge lifetime Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:56   ` [Freedreno] " Kuogee Hsieh
2022-09-22 19:56     ` Kuogee Hsieh
2022-09-13  8:53 ` [PATCH v2 08/10] drm/msm/hdmi: fix IRQ lifetime Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:55   ` Kuogee Hsieh
2022-09-22 19:55     ` Kuogee Hsieh
2022-09-23  6:17     ` Johan Hovold
2022-09-23  6:17       ` Johan Hovold
2022-09-13  8:53 ` [PATCH v2 09/10] drm/msm/dp: drop modeset sanity checks Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:57   ` Kuogee Hsieh
2022-09-22 19:57     ` Kuogee Hsieh
2022-09-26 18:17   ` Abhinav Kumar
2022-09-26 18:17     ` Abhinav Kumar
2022-09-27  7:14     ` Johan Hovold
2022-09-27  7:14       ` Johan Hovold
2022-09-27 18:42       ` Abhinav Kumar
2022-09-27 18:42         ` Abhinav Kumar
2022-09-28 12:24         ` Johan Hovold
2022-09-28 12:24           ` Johan Hovold
2022-09-28 15:33           ` Abhinav Kumar
2022-09-28 15:33             ` Abhinav Kumar
2022-09-28 15:57             ` Johan Hovold
2022-09-28 15:57               ` Johan Hovold
2022-09-13  8:53 ` [PATCH v2 10/10] drm/msm/dsi: " Johan Hovold
2022-09-13  8:53   ` Johan Hovold
2022-09-22 19:58   ` [Freedreno] " Kuogee Hsieh
2022-09-22 19:58     ` Kuogee Hsieh
2022-09-26 18:21   ` Abhinav Kumar
2022-09-26 18:21     ` Abhinav Kumar
2022-09-27  7:16     ` Johan Hovold
2022-09-27  7:16       ` Johan Hovold
2022-09-27 18:44       ` Abhinav Kumar
2022-09-27 18:44         ` Abhinav Kumar
2022-09-13 20:23 ` [PATCH v2 00/10] drm/msm: probe deferral fixes Steev Klimaszewski
2022-09-13 20:23   ` Steev Klimaszewski
2022-09-14  6:01   ` Johan Hovold
2022-09-14  6:01     ` Johan Hovold
2022-09-16 16:43     ` Steev Klimaszewski
2022-09-16 16:43       ` Steev Klimaszewski
2022-09-20  9:06 ` Johan Hovold
2022-09-20  9:06   ` Johan Hovold
2022-10-21  6:27   ` Johan Hovold
2022-10-21  6:27     ` Johan Hovold
2022-10-21 16:05     ` Abhinav Kumar
2022-10-21 16:05       ` Abhinav Kumar
2022-10-24 11:33       ` Johan Hovold
2022-10-24 11:33         ` Johan Hovold
2022-10-24 15:01         ` Rob Clark
2022-10-24 15:01           ` Rob Clark

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.