[PATCH v2 0/2] drm/logicvc: Avoid UAF in DRM object management

[PATCH v2 0/2] drm/logicvc: Avoid UAF in DRM object management

[Romain Gantois]

Hi everyone, this is version two of my series which fixes some memory
management issues in the logicvc-drm driver.

Patch 1/2 migrates the driver to drmm to avoid accessing DRM objects after
they have been freed by devm.

Patch 2/2 uses the unplug mechanism to ensure that DRM objects aren't
accessed after the DRM device is removed.

Best Regards,

Romain

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
Changes in v2:
- Added protection of DRM device resources after removal using drm_dev_enter()
- Link to v1: https://patch.msgid.link/20260601-logicvc-uaf-v1-1-8c9ca5b3429c@bootlin.com

To: Paul Kocialkowski <paulk@sys-base.io>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
Cc: Jason Xiang <jx@jasonxiang.net>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org

---
Romain Gantois (2):
      drm/logicvc: Avoid use-after-free with devm_kzalloc()
      drm/logicvc: Avoid using DRM resources after device is unplugged

 drivers/gpu/drm/logicvc/logicvc_crtc.c      |  52 ++++++----
 drivers/gpu/drm/logicvc/logicvc_drm.c       |   9 +-
 drivers/gpu/drm/logicvc/logicvc_interface.c |  61 +++++------
 drivers/gpu/drm/logicvc/logicvc_layer.c     | 153 +++++++++++++++-------------
 4 files changed, 156 insertions(+), 119 deletions(-)
---
base-commit: 44e151be23deb788d9f6124de93823faf6e04e99
change-id: 20260526-logicvc-uaf-eab103f0d0de

Best regards,
--  
Romain Gantois <romain.gantois@bootlin.com>

[PATCH v2 1/2] drm/logicvc: Avoid use-after-free with devm_kzalloc()

[Romain Gantois]

The logicvc driver calls drm_universal_plane_init(),
drm_crtc_init_with_planes(), and drm_encoder_alloc(). These functions
should not be called with structs allocated with devm_kzalloc(), as this
can lead to use-after-free bugs. In fact, a use-after-free caused by this
has been observed on a v6.6 kernel.

Use DRM-managed allocations instead for panel, CRTC and encoder objects.

Found using KASAN.

Fixes: efeeaefe9be56 ("drm: Add support for the LogiCVC display controller")
Cc: stable@vger.kernel.org
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/gpu/drm/logicvc/logicvc_crtc.c      |  17 ++---
 drivers/gpu/drm/logicvc/logicvc_interface.c |  49 +++++--------
 drivers/gpu/drm/logicvc/logicvc_layer.c     | 105 +++++++++++++---------------
 3 files changed, 75 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/logicvc/logicvc_crtc.c b/drivers/gpu/drm/logicvc/logicvc_crtc.c
index 43a675d03808f..3a4c347eaa648 100644
--- a/drivers/gpu/drm/logicvc/logicvc_crtc.c
+++ b/drivers/gpu/drm/logicvc/logicvc_crtc.c
@@ -13,6 +13,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_print.h>
 #include <drm/drm_vblank.h>
 
@@ -214,7 +215,6 @@ static void logicvc_crtc_disable_vblank(struct drm_crtc *drm_crtc)
 
 static const struct drm_crtc_funcs logicvc_crtc_funcs = {
 	.reset			= drm_atomic_helper_crtc_reset,
-	.destroy		= drm_crtc_cleanup,
 	.set_config		= drm_atomic_helper_set_config,
 	.page_flip		= drm_atomic_helper_page_flip,
 	.atomic_duplicate_state	= drm_atomic_helper_crtc_duplicate_state,
@@ -250,11 +250,6 @@ int logicvc_crtc_init(struct logicvc_drm *logicvc)
 	struct device_node *of_node = dev->of_node;
 	struct logicvc_crtc *crtc;
 	struct logicvc_layer *layer_primary;
-	int ret;
-
-	crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
-	if (!crtc)
-		return -ENOMEM;
 
 	layer_primary = logicvc_layer_get_primary(logicvc);
 	if (!layer_primary) {
@@ -262,12 +257,12 @@ int logicvc_crtc_init(struct logicvc_drm *logicvc)
 		return -EINVAL;
 	}
 
-	ret = drm_crtc_init_with_planes(drm_dev, &crtc->drm_crtc,
-					&layer_primary->drm_plane, NULL,
-					&logicvc_crtc_funcs, NULL);
-	if (ret) {
+	crtc = drmm_crtc_alloc_with_planes(drm_dev, struct logicvc_crtc,
+					   drm_crtc, &layer_primary->drm_plane,
+					   NULL, &logicvc_crtc_funcs, NULL);
+	if (IS_ERR(crtc)) {
 		drm_err(drm_dev, "Failed to initialize CRTC\n");
-		return ret;
+		return PTR_ERR(crtc);
 	}
 
 	drm_crtc_helper_add(&crtc->drm_crtc, &logicvc_crtc_helper_funcs);
diff --git a/drivers/gpu/drm/logicvc/logicvc_interface.c b/drivers/gpu/drm/logicvc/logicvc_interface.c
index 689049d395c0d..0d037f37b950f 100644
--- a/drivers/gpu/drm/logicvc/logicvc_interface.c
+++ b/drivers/gpu/drm/logicvc/logicvc_interface.c
@@ -12,6 +12,7 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
@@ -60,10 +61,6 @@ static const struct drm_encoder_helper_funcs logicvc_encoder_helper_funcs = {
 	.disable		= logicvc_encoder_disable,
 };
 
-static const struct drm_encoder_funcs logicvc_encoder_funcs = {
-	.destroy		= drm_encoder_cleanup,
-};
-
 static int logicvc_connector_get_modes(struct drm_connector *drm_connector)
 {
 	struct logicvc_interface *interface =
@@ -84,7 +81,6 @@ static const struct drm_connector_helper_funcs logicvc_connector_helper_funcs =
 static const struct drm_connector_funcs logicvc_connector_funcs = {
 	.reset			= drm_atomic_helper_connector_reset,
 	.fill_modes		= drm_helper_probe_single_connector_modes,
-	.destroy		= drm_connector_cleanup,
 	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
 };
@@ -147,36 +143,35 @@ int logicvc_interface_init(struct logicvc_drm *logicvc)
 	int encoder_type = logicvc_interface_encoder_type(logicvc);
 	int connector_type = logicvc_interface_connector_type(logicvc);
 	bool native_connector = logicvc_interface_native_connector(logicvc);
+	struct drm_bridge *bridge;
+	struct drm_panel *panel;
 	int ret;
 
-	interface = devm_kzalloc(dev, sizeof(*interface), GFP_KERNEL);
-	if (!interface) {
-		ret = -ENOMEM;
-		goto error_early;
-	}
-
-	ret = drm_of_find_panel_or_bridge(of_node, 0, 0, &interface->drm_panel,
-					  &interface->drm_bridge);
+	ret = drm_of_find_panel_or_bridge(of_node, 0, 0, &panel,
+					  &bridge);
 	if (ret == -EPROBE_DEFER)
-		goto error_early;
+		return ret;
 
-	ret = drm_encoder_init(drm_dev, &interface->drm_encoder,
-			       &logicvc_encoder_funcs, encoder_type, NULL);
-	if (ret) {
+	interface = drmm_encoder_alloc(drm_dev, struct logicvc_interface, drm_encoder,
+				       NULL, encoder_type, NULL);
+	if (IS_ERR(interface)) {
 		drm_err(drm_dev, "Failed to initialize encoder\n");
-		goto error_early;
+		return PTR_ERR(interface);
 	}
 
+	interface->drm_panel = panel;
+	interface->drm_bridge = bridge;
+
 	drm_encoder_helper_add(&interface->drm_encoder,
 			       &logicvc_encoder_helper_funcs);
 
 	if (native_connector || interface->drm_panel) {
-		ret = drm_connector_init(drm_dev, &interface->drm_connector,
-					 &logicvc_connector_funcs,
-					 connector_type);
+		ret = drmm_connector_init(drm_dev, &interface->drm_connector,
+					  &logicvc_connector_funcs,
+					  connector_type, NULL);
 		if (ret) {
 			drm_err(drm_dev, "Failed to initialize connector\n");
-			goto error_encoder;
+			return ret;
 		}
 
 		drm_connector_helper_add(&interface->drm_connector,
@@ -187,7 +182,7 @@ int logicvc_interface_init(struct logicvc_drm *logicvc)
 		if (ret) {
 			drm_err(drm_dev,
 				"Failed to attach connector to encoder\n");
-			goto error_encoder;
+			return ret;
 		}
 	}
 
@@ -197,17 +192,11 @@ int logicvc_interface_init(struct logicvc_drm *logicvc)
 		if (ret) {
 			drm_err(drm_dev,
 				"Failed to attach bridge to encoder\n");
-			goto error_encoder;
+			return ret;
 		}
 	}
 
 	logicvc->interface = interface;
 
 	return 0;
-
-error_encoder:
-	drm_encoder_cleanup(&interface->drm_encoder);
-
-error_early:
-	return ret;
 }
diff --git a/drivers/gpu/drm/logicvc/logicvc_layer.c b/drivers/gpu/drm/logicvc/logicvc_layer.c
index eab4d773f92b6..de1f4a8a61557 100644
--- a/drivers/gpu/drm/logicvc/logicvc_layer.c
+++ b/drivers/gpu/drm/logicvc/logicvc_layer.c
@@ -13,6 +13,7 @@
 #include <drm/drm_fb_dma_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_plane.h>
 #include <drm/drm_print.h>
 
@@ -250,7 +251,6 @@ static struct drm_plane_helper_funcs logicvc_plane_helper_funcs = {
 static const struct drm_plane_funcs logicvc_plane_funcs = {
 	.update_plane		= drm_atomic_helper_update_plane,
 	.disable_plane		= drm_atomic_helper_disable_plane,
-	.destroy		= drm_plane_cleanup,
 	.reset			= drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
@@ -350,16 +350,17 @@ int logicvc_layer_buffer_find_setup(struct logicvc_drm *logicvc,
 	return 0;
 }
 
-static struct logicvc_layer_formats *logicvc_layer_formats_lookup(struct logicvc_layer *layer)
+static struct logicvc_layer_formats *
+logicvc_layer_formats_lookup(struct logicvc_layer_config *config)
 {
 	bool alpha;
 	unsigned int i = 0;
 
-	alpha = (layer->config.alpha_mode == LOGICVC_LAYER_ALPHA_PIXEL);
+	alpha = (config->alpha_mode == LOGICVC_LAYER_ALPHA_PIXEL);
 
 	while (logicvc_layer_formats[i].formats) {
-		if (logicvc_layer_formats[i].colorspace == layer->config.colorspace &&
-		    logicvc_layer_formats[i].depth == layer->config.depth &&
+		if (logicvc_layer_formats[i].colorspace == config->colorspace &&
+		    logicvc_layer_formats[i].depth == config->depth &&
 		    logicvc_layer_formats[i].alpha == alpha)
 			return &logicvc_layer_formats[i];
 
@@ -380,10 +381,9 @@ static unsigned int logicvc_layer_formats_count(struct logicvc_layer_formats *fo
 }
 
 static int logicvc_layer_config_parse(struct logicvc_drm *logicvc,
-				      struct logicvc_layer *layer)
+				      struct device_node *of_node,
+				      struct logicvc_layer_config *config)
 {
-	struct device_node *of_node = layer->of_node;
-	struct logicvc_layer_config *config = &layer->config;
 	int ret;
 
 	logicvc_of_property_parse_bool(of_node,
@@ -458,11 +458,30 @@ struct logicvc_layer *logicvc_layer_get_primary(struct logicvc_drm *logicvc)
 	return logicvc_layer_get_from_type(logicvc, DRM_PLANE_TYPE_PRIMARY);
 }
 
+static void logicvc_layer_set_config(struct logicvc_layer *layer,
+				     struct logicvc_layer_config *config)
+{
+	layer->config.colorspace = config->colorspace;
+	layer->config.depth = config->depth;
+	layer->config.alpha_mode = config->alpha_mode;
+	layer->config.base_offset = config->base_offset;
+	layer->config.buffer_offset = config->buffer_offset;
+	layer->config.primary = config->primary;
+}
+
+static void logicvc_layer_fini(struct drm_device *drm_dev,
+			       void *data)
+{
+	struct logicvc_layer *layer = data;
+
+	list_del(&layer->list);
+}
+
 static int logicvc_layer_init(struct logicvc_drm *logicvc,
 			      struct device_node *of_node, u32 index)
 {
 	struct drm_device *drm_dev = &logicvc->drm_dev;
-	struct device *dev = drm_dev->dev;
+	struct logicvc_layer_config config = { 0 };
 	struct logicvc_layer *layer = NULL;
 	struct logicvc_layer_formats *formats;
 	unsigned int formats_count;
@@ -470,28 +489,18 @@ static int logicvc_layer_init(struct logicvc_drm *logicvc,
 	unsigned int zpos;
 	int ret;
 
-	layer = devm_kzalloc(dev, sizeof(*layer), GFP_KERNEL);
-	if (!layer) {
-		ret = -ENOMEM;
-		goto error;
-	}
-
-	layer->of_node = of_node;
-	layer->index = index;
-
-	ret = logicvc_layer_config_parse(logicvc, layer);
+	ret = logicvc_layer_config_parse(logicvc, of_node, &config);
 	if (ret) {
 		drm_err(drm_dev, "Failed to parse config for layer #%d\n",
 			index);
-		goto error;
+		return ret;
 	}
 
-	formats = logicvc_layer_formats_lookup(layer);
+	formats = logicvc_layer_formats_lookup(&config);
 	if (!formats) {
 		drm_err(drm_dev, "Failed to lookup formats for layer #%d\n",
 			index);
-		ret = -EINVAL;
-		goto error;
+		return -EINVAL;
 	}
 
 	formats_count = logicvc_layer_formats_count(formats);
@@ -511,24 +520,27 @@ static int logicvc_layer_init(struct logicvc_drm *logicvc,
 		regmap_write(logicvc->regmap, LOGICVC_BACKGROUND_COLOR_REG,
 			     background);
 
-		devm_kfree(dev, layer);
-
 		return 0;
 	}
 
-	if (layer->config.primary)
+	if (config.primary)
 		type = DRM_PLANE_TYPE_PRIMARY;
 	else
 		type = DRM_PLANE_TYPE_OVERLAY;
 
-	ret = drm_universal_plane_init(drm_dev, &layer->drm_plane, 0,
-				       &logicvc_plane_funcs, formats->formats,
-				       formats_count, NULL, type, NULL);
-	if (ret) {
+	layer = drmm_universal_plane_alloc(drm_dev, struct logicvc_layer,
+					   drm_plane, 0, &logicvc_plane_funcs,
+					   formats->formats, formats_count,
+					   NULL, type, NULL);
+	if (IS_ERR(layer)) {
 		drm_err(drm_dev, "Failed to initialize layer plane\n");
-		return ret;
+		return PTR_ERR(layer);
 	}
 
+	layer->of_node = of_node;
+	layer->index = index;
+	logicvc_layer_set_config(layer, &config);
+
 	drm_plane_helper_add(&layer->drm_plane, &logicvc_plane_helper_funcs);
 
 	zpos = logicvc->config.layers_count - index - 1;
@@ -545,22 +557,13 @@ static int logicvc_layer_init(struct logicvc_drm *logicvc,
 
 	list_add_tail(&layer->list, &logicvc->layers_list);
 
-	return 0;
-
-error:
-	if (layer)
-		devm_kfree(dev, layer);
-
-	return ret;
-}
+	ret = drmm_add_action_or_reset(drm_dev, logicvc_layer_fini,
+				       layer);
+	if (ret)
+		return ret;
 
-static void logicvc_layer_fini(struct logicvc_drm *logicvc,
-			       struct logicvc_layer *layer)
-{
-	struct device *dev = logicvc->drm_dev.dev;
 
-	list_del(&layer->list);
-	devm_kfree(dev, layer);
+	return 0;
 }
 
 void logicvc_layers_attach_crtc(struct logicvc_drm *logicvc)
@@ -584,14 +587,12 @@ int logicvc_layers_init(struct logicvc_drm *logicvc)
 	struct device_node *layer_node = NULL;
 	struct device_node *layers_node;
 	struct logicvc_layer *layer;
-	struct logicvc_layer *next;
 	int ret = 0;
 
 	layers_node = of_get_child_by_name(of_node, "layers");
 	if (!layers_node) {
 		drm_err(drm_dev, "No layers node found in the description\n");
-		ret = -ENODEV;
-		goto error;
+		return -ENODEV;
 	}
 
 	for_each_child_of_node(layers_node, layer_node) {
@@ -614,17 +615,11 @@ int logicvc_layers_init(struct logicvc_drm *logicvc)
 		ret = logicvc_layer_init(logicvc, layer_node, index);
 		if (ret) {
 			of_node_put(layers_node);
-			goto error;
+			return ret;
 		}
 	}
 
 	of_node_put(layers_node);
 
 	return 0;
-
-error:
-	list_for_each_entry_safe(layer, next, &logicvc->layers_list, list)
-		logicvc_layer_fini(logicvc, layer);
-
-	return ret;
 }

-- 
2.54.0

[PATCH v2 2/2] drm/logicvc: Avoid using DRM resources after device is unplugged

[Romain Gantois]

Some DRM resources such as plane, CRTC or encoder objects could remain in
use after the DRM device is removed. Use the drm_dev_enter/exit() mechanism
to ensure that the DRM device is not unplugged before using its resources.

Fixes: efeeaefe9be56 ("drm: Add support for the LogiCVC display controller")                                                                        │
Cc: stable@vger.kernel.org
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/gpu/drm/logicvc/logicvc_crtc.c      | 35 ++++++++++++++++-----
 drivers/gpu/drm/logicvc/logicvc_drm.c       |  9 +++++-
 drivers/gpu/drm/logicvc/logicvc_interface.c | 12 ++++++++
 drivers/gpu/drm/logicvc/logicvc_layer.c     | 48 ++++++++++++++++++++---------
 4 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/logicvc/logicvc_crtc.c b/drivers/gpu/drm/logicvc/logicvc_crtc.c
index 3a4c347eaa648..f3a224a883b2f 100644
--- a/drivers/gpu/drm/logicvc/logicvc_crtc.c
+++ b/drivers/gpu/drm/logicvc/logicvc_crtc.c
@@ -40,10 +40,15 @@ static void logicvc_crtc_atomic_begin(struct drm_crtc *drm_crtc,
 				      struct drm_atomic_state *state)
 {
 	struct logicvc_crtc *crtc = logicvc_crtc(drm_crtc);
-	struct drm_crtc_state *old_state =
-		drm_atomic_get_old_crtc_state(state, drm_crtc);
 	struct drm_device *drm_dev = drm_crtc->dev;
+	struct drm_crtc_state *old_state;
 	unsigned long flags;
+	int idx;
+
+	if (!drm_dev_enter(drm_dev, &idx))
+		return;
+
+	old_state = drm_atomic_get_old_crtc_state(state, drm_crtc);
 
 	/*
 	 * We need to grab the pending event here if vblank was already enabled
@@ -58,6 +63,8 @@ static void logicvc_crtc_atomic_begin(struct drm_crtc *drm_crtc,
 
 		spin_unlock_irqrestore(&drm_dev->event_lock, flags);
 	}
+
+	drm_dev_exit(idx);
 }
 
 static void logicvc_crtc_atomic_enable(struct drm_crtc *drm_crtc,
@@ -65,17 +72,23 @@ static void logicvc_crtc_atomic_enable(struct drm_crtc *drm_crtc,
 {
 	struct logicvc_crtc *crtc = logicvc_crtc(drm_crtc);
 	struct logicvc_drm *logicvc = logicvc_drm(drm_crtc->dev);
-	struct drm_crtc_state *old_state =
-		drm_atomic_get_old_crtc_state(state, drm_crtc);
-	struct drm_crtc_state *new_state =
-		drm_atomic_get_new_crtc_state(state, drm_crtc);
-	struct drm_display_mode *mode = &new_state->adjusted_mode;
 
 	struct drm_device *drm_dev = drm_crtc->dev;
+	struct drm_crtc_state *old_state;
+	struct drm_crtc_state *new_state;
 	unsigned int hact, hfp, hsl, hbp;
 	unsigned int vact, vfp, vsl, vbp;
+	struct drm_display_mode *mode;
 	unsigned long flags;
 	u32 ctrl;
+	int idx;
+
+	if (!drm_dev_enter(drm_dev, &idx))
+		return;
+
+	old_state = drm_atomic_get_old_crtc_state(state, drm_crtc);
+	new_state = drm_atomic_get_new_crtc_state(state, drm_crtc);
+	mode = &new_state->adjusted_mode;
 
 	/* Timings */
 
@@ -148,6 +161,8 @@ static void logicvc_crtc_atomic_enable(struct drm_crtc *drm_crtc,
 		drm_crtc->state->event = NULL;
 		spin_unlock_irqrestore(&drm_dev->event_lock, flags);
 	}
+
+	drm_dev_exit(idx);
 }
 
 static void logicvc_crtc_atomic_disable(struct drm_crtc *drm_crtc,
@@ -155,6 +170,10 @@ static void logicvc_crtc_atomic_disable(struct drm_crtc *drm_crtc,
 {
 	struct logicvc_drm *logicvc = logicvc_drm(drm_crtc->dev);
 	struct drm_device *drm_dev = drm_crtc->dev;
+	int idx;
+
+	if (!drm_dev_enter(drm_dev, &idx))
+		return;
 
 	drm_crtc_vblank_off(drm_crtc);
 
@@ -180,6 +199,8 @@ static void logicvc_crtc_atomic_disable(struct drm_crtc *drm_crtc,
 		drm_crtc->state->event = NULL;
 		spin_unlock_irq(&drm_dev->event_lock);
 	}
+
+	drm_dev_exit(idx);
 }
 
 static const struct drm_crtc_helper_funcs logicvc_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c b/drivers/gpu/drm/logicvc/logicvc_drm.c
index bbebf4fc7f51a..2112646386e36 100644
--- a/drivers/gpu/drm/logicvc/logicvc_drm.c
+++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
@@ -71,6 +71,7 @@ static irqreturn_t logicvc_drm_irq_handler(int irq, void *data)
 	struct logicvc_drm *logicvc = data;
 	irqreturn_t ret = IRQ_NONE;
 	u32 stat = 0;
+	int idx;
 
 	/* Get pending interrupt sources. */
 	regmap_read(logicvc->regmap, LOGICVC_INT_STAT_REG, &stat);
@@ -79,8 +80,14 @@ static irqreturn_t logicvc_drm_irq_handler(int irq, void *data)
 	regmap_write(logicvc->regmap, LOGICVC_INT_STAT_REG, stat);
 
 	if (stat & LOGICVC_INT_STAT_V_SYNC) {
+		/* DRM device could be unplugged. */
+		if (!drm_dev_enter(&logicvc->drm_dev, &idx))
+			return ret;
+
 		logicvc_crtc_vblank_handler(logicvc);
 		ret = IRQ_HANDLED;
+
+		drm_dev_exit(idx);
 	}
 
 	return ret;
@@ -463,7 +470,7 @@ static void logicvc_drm_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct drm_device *drm_dev = &logicvc->drm_dev;
 
-	drm_dev_unregister(drm_dev);
+	drm_dev_unplug(drm_dev);
 	drm_atomic_helper_shutdown(drm_dev);
 
 	logicvc_mode_fini(logicvc);
diff --git a/drivers/gpu/drm/logicvc/logicvc_interface.c b/drivers/gpu/drm/logicvc/logicvc_interface.c
index 0d037f37b950f..aa13338a29535 100644
--- a/drivers/gpu/drm/logicvc/logicvc_interface.c
+++ b/drivers/gpu/drm/logicvc/logicvc_interface.c
@@ -34,6 +34,10 @@ static void logicvc_encoder_enable(struct drm_encoder *drm_encoder)
 	struct logicvc_drm *logicvc = logicvc_drm(drm_encoder->dev);
 	struct logicvc_interface *interface =
 		logicvc_interface_from_drm_encoder(drm_encoder);
+	int idx;
+
+	if (!drm_dev_enter(drm_encoder->dev, &idx))
+		return;
 
 	regmap_update_bits(logicvc->regmap, LOGICVC_POWER_CTRL_REG,
 			   LOGICVC_POWER_CTRL_VIDEO_ENABLE,
@@ -43,17 +47,25 @@ static void logicvc_encoder_enable(struct drm_encoder *drm_encoder)
 		drm_panel_prepare(interface->drm_panel);
 		drm_panel_enable(interface->drm_panel);
 	}
+
+	drm_dev_exit(idx);
 }
 
 static void logicvc_encoder_disable(struct drm_encoder *drm_encoder)
 {
 	struct logicvc_interface *interface =
 		logicvc_interface_from_drm_encoder(drm_encoder);
+	int idx;
+
+	if (!drm_dev_enter(drm_encoder->dev, &idx))
+		return;
 
 	if (interface->drm_panel) {
 		drm_panel_disable(interface->drm_panel);
 		drm_panel_unprepare(interface->drm_panel);
 	}
+
+	drm_dev_exit(idx);
 }
 
 static const struct drm_encoder_helper_funcs logicvc_encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/logicvc/logicvc_layer.c b/drivers/gpu/drm/logicvc/logicvc_layer.c
index de1f4a8a61557..51fb68e642adc 100644
--- a/drivers/gpu/drm/logicvc/logicvc_layer.c
+++ b/drivers/gpu/drm/logicvc/logicvc_layer.c
@@ -10,6 +10,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_blend.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_fb_dma_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
@@ -87,25 +88,32 @@ static int logicvc_plane_atomic_check(struct drm_plane *drm_plane,
 	struct drm_device *drm_dev = drm_plane->dev;
 	struct logicvc_layer *layer = logicvc_layer(drm_plane);
 	struct logicvc_drm *logicvc = logicvc_drm(drm_dev);
-	struct drm_plane_state *new_state =
-		drm_atomic_get_new_plane_state(state, drm_plane);
+	struct drm_plane_state *new_state;
 	struct drm_crtc_state *crtc_state;
 	int min_scale, max_scale;
 	bool can_position;
-	int ret;
+	int idx, ret = 0;
+
+	if (!drm_dev_enter(drm_dev, &idx))
+		return -ENODEV;
+
+	new_state = drm_atomic_get_new_plane_state(state, drm_plane);
 
 	if (!new_state->crtc)
-		return 0;
+		goto out_exit;
 
 	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
 						   new_state->crtc);
-	if (WARN_ON(!crtc_state))
-		return -EINVAL;
+	if (WARN_ON(!crtc_state)) {
+		ret = -EINVAL;
+		goto out_exit;
+	}
 
 	if (new_state->crtc_x < 0 || new_state->crtc_y < 0) {
 		drm_err(drm_dev,
 			"Negative on-CRTC positions are not supported.\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_exit;
 	}
 
 	if (!logicvc->caps->layer_address) {
@@ -113,7 +121,7 @@ static int logicvc_plane_atomic_check(struct drm_plane *drm_plane,
 						      NULL);
 		if (ret) {
 			drm_err(drm_dev, "No viable setup for buffer found.\n");
-			return ret;
+			goto out_exit;
 		}
 	}
 
@@ -127,12 +135,12 @@ static int logicvc_plane_atomic_check(struct drm_plane *drm_plane,
 	ret = drm_atomic_helper_check_plane_state(new_state, crtc_state,
 						  min_scale, max_scale,
 						  can_position, true);
-	if (ret) {
+	if (ret)
 		drm_err(drm_dev, "Invalid plane state\n\n");
-		return ret;
-	}
 
-	return 0;
+out_exit:
+	drm_dev_exit(idx);
+	return ret;
 }
 
 static void logicvc_plane_atomic_update(struct drm_plane *drm_plane,
@@ -141,15 +149,21 @@ static void logicvc_plane_atomic_update(struct drm_plane *drm_plane,
 	struct logicvc_layer *layer = logicvc_layer(drm_plane);
 	struct logicvc_drm *logicvc = logicvc_drm(drm_plane->dev);
 	struct drm_device *drm_dev = &logicvc->drm_dev;
-	struct drm_plane_state *new_state =
-		drm_atomic_get_new_plane_state(state, drm_plane);
 	struct drm_crtc *drm_crtc = &logicvc->crtc->drm_crtc;
 	struct drm_display_mode *mode = &drm_crtc->state->adjusted_mode;
-	struct drm_framebuffer *fb = new_state->fb;
 	struct logicvc_layer_buffer_setup setup = {};
+	struct drm_plane_state *new_state;
+	struct drm_framebuffer *fb;
 	u32 index = layer->index;
+	int idx;
 	u32 reg;
 
+	if (!drm_dev_enter(drm_dev, &idx))
+		return;
+
+	new_state = drm_atomic_get_new_plane_state(state, drm_plane);
+	fb = new_state->fb;
+
 	/* Layer dimensions */
 
 	regmap_write(logicvc->regmap, LOGICVC_LAYER_WIDTH_REG(index),
@@ -230,6 +244,8 @@ static void logicvc_plane_atomic_update(struct drm_plane *drm_plane,
 	reg |= LOGICVC_LAYER_CTRL_COLOR_KEY_DISABLE;
 
 	regmap_write(logicvc->regmap, LOGICVC_LAYER_CTRL_REG(index), reg);
+
+	drm_dev_exit(idx);
 }
 
 static void logicvc_plane_atomic_disable(struct drm_plane *drm_plane,
@@ -239,6 +255,8 @@ static void logicvc_plane_atomic_disable(struct drm_plane *drm_plane,
 	struct logicvc_drm *logicvc = logicvc_drm(drm_plane->dev);
 	u32 index = layer->index;
 
+	/* No need for drm_dev_enter() here. The regmap outlives the DRM device. */
+
 	regmap_write(logicvc->regmap, LOGICVC_LAYER_CTRL_REG(index), 0);
 }
 

-- 
2.54.0

Re: [PATCH v2 1/2] drm/logicvc: Avoid use-after-free with devm_kzalloc()

[Maxime Ripard]

On Tue, 30 Jun 2026 11:10:10 +0200, Romain Gantois wrote:
> The logicvc driver calls drm_universal_plane_init(),
> drm_crtc_init_with_planes(), and drm_encoder_alloc(). These functions
> should not be called with structs allocated with devm_kzalloc(), as this
> can lead to use-after-free bugs. In fact, a use-after-free caused by this
> has been observed on a v6.6 kernel.
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v2 2/2] drm/logicvc: Avoid using DRM resources after device is unplugged

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5757 bytes --]

On Tue, Jun 30, 2026 at 11:10:11AM +0200, Romain Gantois wrote:
> Some DRM resources such as plane, CRTC or encoder objects could remain in
> use after the DRM device is removed. Use the drm_dev_enter/exit() mechanism
> to ensure that the DRM device is not unplugged before using its resources.
> 
> Fixes: efeeaefe9be56 ("drm: Add support for the LogiCVC display controller")                                                                        │
> Cc: stable@vger.kernel.org
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>  drivers/gpu/drm/logicvc/logicvc_crtc.c      | 35 ++++++++++++++++-----
>  drivers/gpu/drm/logicvc/logicvc_drm.c       |  9 +++++-
>  drivers/gpu/drm/logicvc/logicvc_interface.c | 12 ++++++++
>  drivers/gpu/drm/logicvc/logicvc_layer.c     | 48 ++++++++++++++++++++---------
>  4 files changed, 81 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/logicvc/logicvc_crtc.c b/drivers/gpu/drm/logicvc/logicvc_crtc.c
> index 3a4c347eaa648..f3a224a883b2f 100644
> --- a/drivers/gpu/drm/logicvc/logicvc_crtc.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_crtc.c
> @@ -40,10 +40,15 @@ static void logicvc_crtc_atomic_begin(struct drm_crtc *drm_crtc,
>  				      struct drm_atomic_state *state)
>  {
>  	struct logicvc_crtc *crtc = logicvc_crtc(drm_crtc);
> -	struct drm_crtc_state *old_state =
> -		drm_atomic_get_old_crtc_state(state, drm_crtc);
>  	struct drm_device *drm_dev = drm_crtc->dev;
> +	struct drm_crtc_state *old_state;
>  	unsigned long flags;
> +	int idx;
> +
> +	if (!drm_dev_enter(drm_dev, &idx))
> +		return;
> +
> +	old_state = drm_atomic_get_old_crtc_state(state, drm_crtc);

You don't have to move the state around here, the states are always safe
to access in the atomic callbacks.

>  	/*
>  	 * We need to grab the pending event here if vblank was already enabled
> @@ -58,6 +63,8 @@ static void logicvc_crtc_atomic_begin(struct drm_crtc *drm_crtc,
>  
>  		spin_unlock_irqrestore(&drm_dev->event_lock, flags);
>  	}
> +
> +	drm_dev_exit(idx);

Only the device resources (ie, clocks, registers, etc. ) need to be
guarded. Any DRM facing resource can still used, and the vblank here
should still be signalled.

>  }
>  
>  static void logicvc_crtc_atomic_enable(struct drm_crtc *drm_crtc,
> @@ -65,17 +72,23 @@ static void logicvc_crtc_atomic_enable(struct drm_crtc *drm_crtc,
>  {
>  	struct logicvc_crtc *crtc = logicvc_crtc(drm_crtc);
>  	struct logicvc_drm *logicvc = logicvc_drm(drm_crtc->dev);
> -	struct drm_crtc_state *old_state =
> -		drm_atomic_get_old_crtc_state(state, drm_crtc);
> -	struct drm_crtc_state *new_state =
> -		drm_atomic_get_new_crtc_state(state, drm_crtc);
> -	struct drm_display_mode *mode = &new_state->adjusted_mode;
>  
>  	struct drm_device *drm_dev = drm_crtc->dev;
> +	struct drm_crtc_state *old_state;
> +	struct drm_crtc_state *new_state;
>  	unsigned int hact, hfp, hsl, hbp;
>  	unsigned int vact, vfp, vsl, vbp;
> +	struct drm_display_mode *mode;
>  	unsigned long flags;
>  	u32 ctrl;
> +	int idx;
> +
> +	if (!drm_dev_enter(drm_dev, &idx))
> +		return;
> +
> +	old_state = drm_atomic_get_old_crtc_state(state, drm_crtc);
> +	new_state = drm_atomic_get_new_crtc_state(state, drm_crtc);
> +	mode = &new_state->adjusted_mode;
>  
>  	/* Timings */
>  
> @@ -148,6 +161,8 @@ static void logicvc_crtc_atomic_enable(struct drm_crtc *drm_crtc,
>  		drm_crtc->state->event = NULL;
>  		spin_unlock_irqrestore(&drm_dev->event_lock, flags);
>  	}
> +
> +	drm_dev_exit(idx);
>  }
>  
>  static void logicvc_crtc_atomic_disable(struct drm_crtc *drm_crtc,
> @@ -155,6 +170,10 @@ static void logicvc_crtc_atomic_disable(struct drm_crtc *drm_crtc,
>  {
>  	struct logicvc_drm *logicvc = logicvc_drm(drm_crtc->dev);
>  	struct drm_device *drm_dev = drm_crtc->dev;
> +	int idx;
> +
> +	if (!drm_dev_enter(drm_dev, &idx))
> +		return;
>  
>  	drm_crtc_vblank_off(drm_crtc);
>  
> @@ -180,6 +199,8 @@ static void logicvc_crtc_atomic_disable(struct drm_crtc *drm_crtc,
>  		drm_crtc->state->event = NULL;
>  		spin_unlock_irq(&drm_dev->event_lock);
>  	}
> +
> +	drm_dev_exit(idx);
>  }
>  
>  static const struct drm_crtc_helper_funcs logicvc_crtc_helper_funcs = {
> diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c b/drivers/gpu/drm/logicvc/logicvc_drm.c
> index bbebf4fc7f51a..2112646386e36 100644
> --- a/drivers/gpu/drm/logicvc/logicvc_drm.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
> @@ -71,6 +71,7 @@ static irqreturn_t logicvc_drm_irq_handler(int irq, void *data)
>  	struct logicvc_drm *logicvc = data;
>  	irqreturn_t ret = IRQ_NONE;
>  	u32 stat = 0;
> +	int idx;
>  
>  	/* Get pending interrupt sources. */
>  	regmap_read(logicvc->regmap, LOGICVC_INT_STAT_REG, &stat);

strictly speaking, that regmap read here should be protected, but it's
not going to be executed after remove anyway because the interrupt
handler will be deregistered, so you don't have to bother here.

[...]

>  static void logicvc_plane_atomic_disable(struct drm_plane *drm_plane,
> @@ -239,6 +255,8 @@ static void logicvc_plane_atomic_disable(struct drm_plane *drm_plane,
>  	struct logicvc_drm *logicvc = logicvc_drm(drm_plane->dev);
>  	u32 index = layer->index;
>  
> +	/* No need for drm_dev_enter() here. The regmap outlives the DRM device. */
> +
>  	regmap_write(logicvc->regmap, LOGICVC_LAYER_CTRL_REG(index), 0);

No it doesn't? It's a devm allocated regmap, it's going to be destroyed
at remove. The DRM device will stick around after remove for as long as
there's a userspace application with a fd to it, so the DRM device far
outlives the regmap (and you end up with a dangling pointer to the regmap)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]