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

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

[Romain Gantois]

Hi everyone, this is version three 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 v3:
- Added a required drm_device_enter() section.
- Removed unecessary drm_device_enter() sections.
- Cleared vblank events in atomic callbacks when device is unplugged.
- Link to v2: https://patch.msgid.link/20260630-logicvc-uaf-v2-0-99e881833860@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      |  56 +++++++++---
 drivers/gpu/drm/logicvc/logicvc_drm.c       |   6 +-
 drivers/gpu/drm/logicvc/logicvc_interface.c |  61 ++++++-------
 drivers/gpu/drm/logicvc/logicvc_layer.c     | 133 +++++++++++++++-------------
 4 files changed, 153 insertions(+), 103 deletions(-)
---
base-commit: 44e151be23deb788d9f6124de93823faf6e04e99
change-id: 20260526-logicvc-uaf-eab103f0d0de

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

[PATCH v3 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
Reviewed-by: Maxime Ripard <mripard@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.55.0

[PATCH v3 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      | 39 +++++++++++++++++++++++++++++
 drivers/gpu/drm/logicvc/logicvc_drm.c       |  6 ++++-
 drivers/gpu/drm/logicvc/logicvc_interface.c | 12 +++++++++
 drivers/gpu/drm/logicvc/logicvc_layer.c     | 28 ++++++++++++++++-----
 4 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/logicvc/logicvc_crtc.c b/drivers/gpu/drm/logicvc/logicvc_crtc.c
index 3a4c347eaa648..e2575fa78ab25 100644
--- a/drivers/gpu/drm/logicvc/logicvc_crtc.c
+++ b/drivers/gpu/drm/logicvc/logicvc_crtc.c
@@ -36,6 +36,21 @@ logicvc_crtc_mode_valid(struct drm_crtc *drm_crtc,
 	return 0;
 }
 
+static void logicvc_crtc_drop_any_event(struct drm_device *drm_dev,
+					struct drm_crtc *drm_crtc)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&drm_dev->event_lock, flags);
+
+	if (drm_crtc->state->event) {
+		drm_crtc->state->event = NULL;
+		drm_warn(drm_crtc->dev, "Device is unplugged, ignoring pending vblank event!");
+	}
+
+	spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+}
+
 static void logicvc_crtc_atomic_begin(struct drm_crtc *drm_crtc,
 				      struct drm_atomic_state *state)
 {
@@ -44,6 +59,12 @@ static void logicvc_crtc_atomic_begin(struct drm_crtc *drm_crtc,
 		drm_atomic_get_old_crtc_state(state, drm_crtc);
 	struct drm_device *drm_dev = drm_crtc->dev;
 	unsigned long flags;
+	int idx;
+
+	if (!drm_dev_enter(drm_dev, &idx)) {
+		logicvc_crtc_drop_any_event(drm_dev, drm_crtc);
+		return;
+	}
 
 	/*
 	 * We need to grab the pending event here if vblank was already enabled
@@ -58,6 +79,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,
@@ -76,6 +99,12 @@ static void logicvc_crtc_atomic_enable(struct drm_crtc *drm_crtc,
 	unsigned int vact, vfp, vsl, vbp;
 	unsigned long flags;
 	u32 ctrl;
+	int idx;
+
+	if (!drm_dev_enter(drm_dev, &idx)) {
+		logicvc_crtc_drop_any_event(drm_dev, drm_crtc);
+		return;
+	}
 
 	/* Timings */
 
@@ -148,6 +177,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 +186,12 @@ 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)) {
+		logicvc_crtc_drop_any_event(drm_dev, drm_crtc);
+		return;
+	}
 
 	drm_crtc_vblank_off(drm_crtc);
 
@@ -180,6 +217,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..fb66f5fb67937 100644
--- a/drivers/gpu/drm/logicvc/logicvc_drm.c
+++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
@@ -72,6 +72,10 @@ static irqreturn_t logicvc_drm_irq_handler(int irq, void *data)
 	irqreturn_t ret = IRQ_NONE;
 	u32 stat = 0;
 
+	/* The interrupt handler will be unregistered when the device is
+	 * removed. Therefore, there's no need for drm_dev_enter() here.
+	 */
+
 	/* Get pending interrupt sources. */
 	regmap_read(logicvc->regmap, LOGICVC_INT_STAT_REG, &stat);
 
@@ -463,7 +467,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..4d9bfd57affcf 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>
@@ -92,7 +93,7 @@ static int logicvc_plane_atomic_check(struct drm_plane *drm_plane,
 	struct drm_crtc_state *crtc_state;
 	int min_scale, max_scale;
 	bool can_position;
-	int ret;
+	int idx, ret = 0;
 
 	if (!new_state->crtc)
 		return 0;
@@ -108,12 +109,15 @@ static int logicvc_plane_atomic_check(struct drm_plane *drm_plane,
 		return -EINVAL;
 	}
 
+	if (!drm_dev_enter(drm_dev, &idx))
+		return -ENODEV;
+
 	if (!logicvc->caps->layer_address) {
 		ret = logicvc_layer_buffer_find_setup(logicvc, layer, new_state,
 						      NULL);
 		if (ret) {
 			drm_err(drm_dev, "No viable setup for buffer found.\n");
-			return ret;
+			goto out_exit;
 		}
 	}
 
@@ -127,12 +131,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,
@@ -148,8 +152,12 @@ static void logicvc_plane_atomic_update(struct drm_plane *drm_plane,
 	struct drm_framebuffer *fb = new_state->fb;
 	struct logicvc_layer_buffer_setup setup = {};
 	u32 index = layer->index;
+	int idx;
 	u32 reg;
 
+	if (!drm_dev_enter(drm_dev, &idx))
+		return;
+
 	/* Layer dimensions */
 
 	regmap_write(logicvc->regmap, LOGICVC_LAYER_WIDTH_REG(index),
@@ -230,6 +238,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,
@@ -238,8 +248,14 @@ static void logicvc_plane_atomic_disable(struct drm_plane *drm_plane,
 	struct logicvc_layer *layer = logicvc_layer(drm_plane);
 	struct logicvc_drm *logicvc = logicvc_drm(drm_plane->dev);
 	u32 index = layer->index;
+	int idx;
+
+	if (!drm_dev_enter(&logicvc->drm_dev, &idx))
+		return;
 
 	regmap_write(logicvc->regmap, LOGICVC_LAYER_CTRL_REG(index), 0);
+
+	drm_dev_exit(idx);
 }
 
 static struct drm_plane_helper_funcs logicvc_plane_helper_funcs = {

-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The local variable `bridge` is passed to `drm_of_find_panel_or_bridge()` without initialization, leading to a crash when the function returns an error without setting it.
--

--- Patch [1]: [PATCH v3 1/2] drm/logicvc: Avoid use-after-free with devm_kzalloc() ---
commit 95ef468f51e9bfed648e230bd616663ec7bac106
Author: Romain Gantois <romain.gantois@bootlin.com>

drm/logicvc: Avoid use-after-free with devm_kzalloc()

This commit transitions the panel, CRTC, and encoder objects to DRM-managed
allocations. It removes devm_kzalloc usages in order to fix a use-after-free
bug.

> diff --git a/drivers/gpu/drm/logicvc/logicvc_interface.c b/drivers/gpu/drm/logicvc/logicvc_interface.c
> --- a/drivers/gpu/drm/logicvc/logicvc_interface.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_interface.c
> @@ -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;

[Severity: High]
Are panel and bridge guaranteed to be initialized if this call fails?

When drm_of_find_panel_or_bridge() returns an error like -ENODEV (which
is valid for native DVI connectors without an OF graph), it might not
initialize these variables. Previously, interface was allocated with
devm_kzalloc(), so they were safely initialized to NULL. Now, they are
uninitialized stack variables.

> -	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;

[Severity: High]
Could this assignment cause a crash later during driver initialization?

If drm_of_find_panel_or_bridge() returned -ENODEV, bridge will contain
stack garbage. Later in logicvc_interface_init(), the code evaluates
if (interface->drm_bridge) and calls drm_bridge_attach(), which would
dereference this uninitialized pointer and trigger a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-logicvc-uaf-v3-0-f3b2319d58a6@bootlin.com?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Leaking vblank event and causing userspace to hang indefinitely on device unplug.
- [High] Bailing out early from CRTC disable skips mandatory DRM software state cleanup (`drm_crtc_vblank_off`).
- [High] Calling `drm_dev_unplug()` before `drm_atomic_helper_shutdown()` aborts hardware shutdown, leading to DMA use-after-free and system hangs.
- [High] Omitting `drm_dev_enter()` in the IRQ handler risks unclocked register access and system crashes during device removal.
- [Medium] The external DRM panel is not properly disabled and unprepared upon device unplug.
--

--- Patch [2]: [PATCH v3 2/2] drm/logicvc: Avoid using DRM resources after device is unplugged ---
commit 984a88d7dda613a6824feda7d8e4824b35006bb5
Author: Romain Gantois <romain.gantois@bootlin.com>
Subject: drm/logicvc: Avoid using DRM resources after device is unplugged

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.

> diff --git a/drivers/gpu/drm/logicvc/logicvc_crtc.c b/drivers/gpu/drm/logicvc/logicvc_crtc.c
> index 3a4c347eaa648..e2575fa78ab25 100644
> --- a/drivers/gpu/drm/logicvc/logicvc_crtc.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_crtc.c
> @@ -36,6 +36,21 @@ logicvc_crtc_mode_valid(struct drm_crtc *drm_crtc,
>  	return 0;
>  }
>  
> +static void logicvc_crtc_drop_any_event(struct drm_device *drm_dev,
> +					struct drm_crtc *drm_crtc)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&drm_dev->event_lock, flags);
> +
> +	if (drm_crtc->state->event) {
> +		drm_crtc->state->event = NULL;

[Severity: High]
Does setting this event to NULL leak the drm_pending_vblank_event memory?

Without calling drm_crtc_send_vblank_event(), will userspace compositors
waiting for the page flip completion hang indefinitely during an unplug?

> +		drm_warn(drm_crtc->dev, "Device is unplugged, ignoring pending vblank event!");
> +	}
> +
> +	spin_unlock_irqrestore(&drm_dev->event_lock, flags);
> +}

[ ... ]

> @@ -155,6 +186,12 @@ 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)) {
> +		logicvc_crtc_drop_any_event(drm_dev, drm_crtc);
> +		return;
> +	}

[Severity: High]
If we return early here during an unplug, we skip the call to
drm_crtc_vblank_off(). Does this leave the DRM core's vblank state machine
active, potentially orphaning waitqueues or leaving background works
pending?

>  
>  	drm_crtc_vblank_off(drm_crtc);

[ ... ]

> diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c b/drivers/gpu/drm/logicvc/logicvc_drm.c
> index bbebf4fc7f51a..fb66f5fb67937 100644
> --- a/drivers/gpu/drm/logicvc/logicvc_drm.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
> @@ -72,6 +72,10 @@ static irqreturn_t logicvc_drm_irq_handler(int irq, void *data)
>  	irqreturn_t ret = IRQ_NONE;
>  	u32 stat = 0;
>  
> +	/* The interrupt handler will be unregistered when the device is
> +	 * removed. Therefore, there's no need for drm_dev_enter() here.
> +	 */
> +
>  	/* Get pending interrupt sources. */
>  	regmap_read(logicvc->regmap, LOGICVC_INT_STAT_REG, &stat);

[Severity: High]
Since the interrupt handler relies on devres for unregistration, it remains
active after logicvc_drm_remove() completes.

Inside logicvc_drm_remove(), logicvc_clocks_unprepare() is called which
disables the clocks. If this handler fires afterwards (e.g., from a shared
IRQ line), won't reading LOGICVC_INT_STAT_REG without clocks cause an
imprecise external abort and crash the system?

>  
> @@ -463,7 +467,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);

[Severity: High]
Calling drm_dev_unplug() here sets the device unplugged state, which causes
subsequent drm_dev_enter() checks to fail in functions like
logicvc_plane_atomic_check().

If logicvc_plane_atomic_check() returns -ENODEV, won't this abort the
drm_atomic_helper_shutdown() commit? If the hardware display controller is
never stopped and continues fetching from memory, disabling the clocks later
could cause an AXI bus lockup.

>  
>  	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
> @@ -43,17 +47,25 @@ static void logicvc_encoder_enable(struct drm_encoder *drm_encoder)

[ ... ]

>  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;

[Severity: Medium]
Does returning early here skip disabling and unpreparing the DRM panel?

If the device is unplugged, skipping drm_panel_disable() and
drm_panel_unprepare() could leave the panel permanently powered on with its
regulators and backlight enabled, causing a resource leak.

>  
>  	if (interface->drm_panel) {
>  		drm_panel_disable(interface->drm_panel);
>  		drm_panel_unprepare(interface->drm_panel);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-logicvc-uaf-v3-0-f3b2319d58a6@bootlin.com?part=2