The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] drm/logicvc: Avoid use-after-free with devm_kzalloc()
@ 2026-06-01  6:52 Romain Gantois
  2026-06-01  7:11 ` Maxime Ripard
  0 siblings, 1 reply; 5+ messages in thread
From: Romain Gantois @ 2026-06-01  6:52 UTC (permalink / raw)
  To: Paul Kocialkowski, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: Thomas Petazzoni, Paul Kocialkowski, dri-devel, linux-kernel,
	stable, 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;
 }

---
base-commit: 44e151be23deb788d9f6124de93823faf6e04e99
change-id: 20260526-logicvc-uaf-eab103f0d0de

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


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

* Re: [PATCH] drm/logicvc: Avoid use-after-free with devm_kzalloc()
  2026-06-01  6:52 [PATCH] drm/logicvc: Avoid use-after-free with devm_kzalloc() Romain Gantois
@ 2026-06-01  7:11 ` Maxime Ripard
  2026-06-08 15:41   ` Romain Gantois
  0 siblings, 1 reply; 5+ messages in thread
From: Maxime Ripard @ 2026-06-01  7:11 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Paul Kocialkowski, Maarten Lankhorst, Thomas Zimmermann,
	David Airlie, Simona Vetter, Thomas Petazzoni, Paul Kocialkowski,
	dri-devel, linux-kernel, stable

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

Hi,

On Mon, Jun 01, 2026 at 08:52:44AM +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.
> 
> 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>

You're only partially fixing the issue. You also need to protect any
device resource (register mapping, clocks, etc) are no longer accessed
after the device has been removed, and this is typically done using
drm_dev_enter/exit.

Maxime

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

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

* Re: [PATCH] drm/logicvc: Avoid use-after-free with devm_kzalloc()
  2026-06-01  7:11 ` Maxime Ripard
@ 2026-06-08 15:41   ` Romain Gantois
  2026-06-08 16:19     ` Maxime Ripard
  0 siblings, 1 reply; 5+ messages in thread
From: Romain Gantois @ 2026-06-08 15:41 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Paul Kocialkowski, Maarten Lankhorst, Thomas Zimmermann,
	David Airlie, Simona Vetter, Thomas Petazzoni, Paul Kocialkowski,
	dri-devel, linux-kernel, stable

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

Hi Maxime,

On Monday, 1 June 2026 09:11:21 CEST Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jun 01, 2026 at 08:52:44AM +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.
> > 
> > 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>
> 
> You're only partially fixing the issue. You also need to protect any
> device resource (register mapping, clocks, etc) are no longer accessed
> after the device has been removed, and this is typically done using
> drm_dev_enter/exit.

Sorry there's something which I don't quite understand: is this a new issue 
which is specifically introduced by my changes in this series, or a different 
issue in this driver which isn't handled by my series?

IIUC all I'm doing here is just letting the drmm code handle cleaning up the 
plane, crtc, etc. objects instead of doing it "by hand" with devm_kzalloc. Why 
does this make it necessary to add additional protection of driver resources?

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] drm/logicvc: Avoid use-after-free with devm_kzalloc()
  2026-06-08 15:41   ` Romain Gantois
@ 2026-06-08 16:19     ` Maxime Ripard
  2026-06-09  7:16       ` Romain Gantois
  0 siblings, 1 reply; 5+ messages in thread
From: Maxime Ripard @ 2026-06-08 16:19 UTC (permalink / raw)
  To: Romain Gantois
  Cc: Paul Kocialkowski, Maarten Lankhorst, Thomas Zimmermann,
	David Airlie, Simona Vetter, Thomas Petazzoni, Paul Kocialkowski,
	dri-devel, linux-kernel, stable

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

On Mon, Jun 08, 2026 at 05:41:11PM +0200, Romain Gantois wrote:
> Hi Maxime,
> 
> On Monday, 1 June 2026 09:11:21 CEST Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Jun 01, 2026 at 08:52:44AM +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.
> > > 
> > > 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>
> > 
> > You're only partially fixing the issue. You also need to protect any
> > device resource (register mapping, clocks, etc) are no longer accessed
> > after the device has been removed, and this is typically done using
> > drm_dev_enter/exit.
> 
> Sorry there's something which I don't quite understand: is this a new issue 
> which is specifically introduced by my changes in this series, or a different 
> issue in this driver which isn't handled by my series?

A bit of both I guess ? :)

My point was that while your commit log claims you avoid use-after-free,
and your patch definitely avoids some, you can still trivially trigger
some.

Whether you want to fix them all at once or prefer to defer it to a
later point in time is equally fine by me, but you need to be aware that
it's not done, and you probably want to have it in the commit log
somewhere too?

> IIUC all I'm doing here is just letting the drmm code handle cleaning up the 
> plane, crtc, etc. objects instead of doing it "by hand" with devm_kzalloc. Why 
> does this make it necessary to add additional protection of driver resources?

It's not necessary, but it's also kind of the same issue. The reason we
need to have drmm over devm is that the driver stays around longer than
its device, so devm-allocated memory would have been freed.

But that's also the case for *any* devm resource, or more generally any
resource linked to that device, so register mappings, clocks,
interrupts, etc.

So yeah, it's a different symptom of the same underlying cause.

Maxime

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

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

* Re: [PATCH] drm/logicvc: Avoid use-after-free with devm_kzalloc()
  2026-06-08 16:19     ` Maxime Ripard
@ 2026-06-09  7:16       ` Romain Gantois
  0 siblings, 0 replies; 5+ messages in thread
From: Romain Gantois @ 2026-06-09  7:16 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Paul Kocialkowski, Maarten Lankhorst, Thomas Zimmermann,
	David Airlie, Simona Vetter, Thomas Petazzoni, Paul Kocialkowski,
	dri-devel, linux-kernel, stable

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

On Monday, 8 June 2026 18:19:04 CEST Maxime Ripard wrote:
> On Mon, Jun 08, 2026 at 05:41:11PM +0200, Romain Gantois wrote:
> > Hi Maxime,
> > 
...
> > > You're only partially fixing the issue. You also need to protect any
> > > device resource (register mapping, clocks, etc) are no longer accessed
> > > after the device has been removed, and this is typically done using
> > > drm_dev_enter/exit.
> > 
> > Sorry there's something which I don't quite understand: is this a new
> > issue
> > which is specifically introduced by my changes in this series, or a
> > different issue in this driver which isn't handled by my series?
> 
> A bit of both I guess ? :)
> 
> My point was that while your commit log claims you avoid use-after-free,
> and your patch definitely avoids some, you can still trivially trigger
> some.
> 
> Whether you want to fix them all at once or prefer to defer it to a
> later point in time is equally fine by me, but you need to be aware that
> it's not done, and you probably want to have it in the commit log
> somewhere too?
> 

Ah ok, I didn't mean to claim that I was fixing any potential UAF in the 
driver. But if there's another one then I'm definitely interested in fixing it 
as well :).

> > IIUC all I'm doing here is just letting the drmm code handle cleaning up
> > the plane, crtc, etc. objects instead of doing it "by hand" with
> > devm_kzalloc. Why does this make it necessary to add additional
> > protection of driver resources?
> 
> It's not necessary, but it's also kind of the same issue. The reason we
> need to have drmm over devm is that the driver stays around longer than
> its device, so devm-allocated memory would have been freed.
> 
> But that's also the case for *any* devm resource, or more generally any
> resource linked to that device, so register mappings, clocks,
> interrupts, etc.
> 
> So yeah, it's a different symptom of the same underlying cause.

Okay, I'll assess the issue and probably make another patch in the same series 
to fix that.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2026-06-09  7:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-01  6:52 [PATCH] drm/logicvc: Avoid use-after-free with devm_kzalloc() Romain Gantois
2026-06-01  7:11 ` Maxime Ripard
2026-06-08 15:41   ` Romain Gantois
2026-06-08 16:19     ` Maxime Ripard
2026-06-09  7:16       ` Romain Gantois

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox