linux-kernel - [PATCH] drm/mipi-dsi: Fix devm unregister & detach

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Wed, 19 Jun 2024 12:07:48 +0300
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, 
 Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
 Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>, 
 Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Subject: [PATCH] drm/mipi-dsi: Fix devm unregister & detach

From: Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>

When a bridge driver uses devm_mipi_dsi_device_register_full() or
devm_mipi_dsi_attach(), the resource management is moved to devres,
which releases the resource automatically when the bridge driver is
unbound.

However, if the DSI host goes away first, the host unregistration code
will automatically detach and unregister any DSI peripherals, without
notifying the devres about it. So when the bridge driver later is
unbound, the resources are released a second time, leading to crash.

Fix this by recording the device that was used when calling the above
mentioned functions into the struct mipi_dsi_device, and when the
unregister or detach is called, remove the devres action if needed.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>
---
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 44 ++++++++++++++++++++++++++++++++----------
 include/drm/drm_mipi_dsi.h     |  6 ++++++
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 795001bb7ff1..a78c4b6cae70 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -237,13 +237,15 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
 }
 EXPORT_SYMBOL(mipi_dsi_device_register_full);
 
+static void mipi_dsi_do_device_unregister(struct mipi_dsi_device *dsi, bool devres);
+
 /**
  * mipi_dsi_device_unregister - unregister MIPI DSI device
  * @dsi: DSI peripheral device
  */
 void mipi_dsi_device_unregister(struct mipi_dsi_device *dsi)
 {
-	device_unregister(&dsi->dev);
+	mipi_dsi_do_device_unregister(dsi, false);
 }
 EXPORT_SYMBOL(mipi_dsi_device_unregister);
 
@@ -251,7 +253,15 @@ static void devm_mipi_dsi_device_unregister(void *arg)
 {
 	struct mipi_dsi_device *dsi = arg;
 
-	mipi_dsi_device_unregister(dsi);
+	mipi_dsi_do_device_unregister(dsi, true);
+}
+
+static void mipi_dsi_do_device_unregister(struct mipi_dsi_device *dsi, bool devres)
+{
+	if (!devres && dsi->devres_register_dev)
+		devm_remove_action(dsi->devres_register_dev, devm_mipi_dsi_device_unregister, dsi);
+
+	device_unregister(&dsi->dev);
 }
 
 /**
@@ -289,6 +299,8 @@ devm_mipi_dsi_device_register_full(struct device *dev,
 	if (ret)
 		return ERR_PTR(ret);
 
+	dsi->devres_register_dev = dev;
+
 	return dsi;
 }
 EXPORT_SYMBOL_GPL(devm_mipi_dsi_device_register_full);
@@ -386,17 +398,35 @@ int mipi_dsi_attach(struct mipi_dsi_device *dsi)
 }
 EXPORT_SYMBOL(mipi_dsi_attach);
 
+static int mipi_dsi_do_detach(struct mipi_dsi_device *dsi, bool devres);
+
 /**
  * mipi_dsi_detach - detach a DSI device from its DSI host
  * @dsi: DSI peripheral
  */
 int mipi_dsi_detach(struct mipi_dsi_device *dsi)
+{
+	return mipi_dsi_do_detach(dsi, false);
+}
+EXPORT_SYMBOL(mipi_dsi_detach);
+
+static void devm_mipi_dsi_detach(void *arg)
+{
+	struct mipi_dsi_device *dsi = arg;
+
+	mipi_dsi_do_detach(dsi, true);
+}
+
+static int mipi_dsi_do_detach(struct mipi_dsi_device *dsi, bool devres)
 {
 	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
 
 	if (WARN_ON(!dsi->attached))
 		return -EINVAL;
 
+	if (!devres && dsi->devres_attach_dev)
+		devm_remove_action(dsi->devres_attach_dev, devm_mipi_dsi_detach, dsi);
+
 	if (!ops || !ops->detach)
 		return -ENOSYS;
 
@@ -404,14 +434,6 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
 
 	return ops->detach(dsi->host, dsi);
 }
-EXPORT_SYMBOL(mipi_dsi_detach);
-
-static void devm_mipi_dsi_detach(void *arg)
-{
-	struct mipi_dsi_device *dsi = arg;
-
-	mipi_dsi_detach(dsi);
-}
 
 /**
  * devm_mipi_dsi_attach - Attach a MIPI-DSI device to its DSI Host
@@ -437,6 +459,8 @@ int devm_mipi_dsi_attach(struct device *dev,
 	if (ret)
 		return ret;
 
+	dsi->devres_attach_dev = dev;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(devm_mipi_dsi_attach);
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 82b1cc434ea3..f68aee6813db 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -181,6 +181,9 @@ struct mipi_dsi_device_info {
  * be set to the real limits of the hardware, zero is only accepted for
  * legacy drivers
  * @dsc: panel/bridge DSC pps payload to be sent
+ * devres_register_dev: device that was used with
+ *			devm_mipi_dsi_device_register_full() or NULL
+ * devres_attach_dev: device that was used with devm_mipi_dsi_attach() or NULL
  */
 struct mipi_dsi_device {
 	struct mipi_dsi_host *host;
@@ -195,6 +198,9 @@ struct mipi_dsi_device {
 	unsigned long hs_rate;
 	unsigned long lp_rate;
 	struct drm_dsc_config *dsc;
+
+	struct device *devres_register_dev;
+	struct device *devres_attach_dev;
 };
 
 #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"

---
base-commit: 6ba59ff4227927d3a8530fc2973b80e94b54d58f
change-id: 20240619-dsi-devres-fix-8d55852b406a

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>