[PATCH] drm/vkms: Fix UAF between connector configfs rmdir and .detect

[PATCH] drm/vkms: Fix UAF between connector configfs rmdir and .detect

[Ibrahim Hashimov]

vkms_connector_detect() walks config->connectors via
vkms_config_for_each_connector(), which expands to a plain
list_for_each_entry() over vkms_config::connectors with no lock and no
RCU protection.

Concurrently, rmdir'ing a connector's configfs directory calls
connector_release(), which takes the configfs device mutex
(connector->dev->lock) and calls vkms_config_destroy_connector(),
which does:

	list_del(&connector_cfg->link);
	kfree(connector_cfg);

Unlike make_crtc_group()/make_plane_group()/make_encoder_group()/
make_connector_group() and the possible_crtcs/possible_encoders
allow_link() callbacks -- all of which refuse the operation with
-EBUSY while the device is enabled -- connector_release() has no such
guard, and in fact cannot be given one: release() runs after configfs
has already committed to removing the directory, so it cannot fail the
rmdir(2) syscall with -EBUSY. A connector's configfs directory can
therefore be removed, and its vkms_config_connector freed, at any time
while the device is live and its DRM connector is still being probed.

vkms_connector_detect() is reached from userspace by writing "detect"
to /sys/class/drm/<card>-<conn>/status (drm_sysfs status_store() ->
drm_helper_probe_single_connector_modes() -> .detect), which runs under
drm_device.mode_config.mutex. Nothing prevents this from running at
the same time as the configfs rmdir above, since the two paths take
entirely different locks (configfs connector->dev->lock vs. DRM's
mode_config.mutex) over the same vkms_config_connector object. Both
sides require local root/CAP_SYS_ADMIN: the vkms configfs tree is
root-owned (0755) and the connector's sysfs status attribute is
root-writable by default, so this is a local-root race, not an
unprivileged-user or remote issue.

The result is a classic race-into-use-after-free: the unlocked iterator
dereferences connector_cfg->connector or follows connector_cfg->link
into memory that connector_release() has already kfree()'d, or reads
connector_cfg->status out of freed memory. This reproduces as a KASAN
slab-use-after-free in vkms_connector_detect() under a detect-hammer
vs. connector-rmdir stress loop.

A tempting minimal fix is to have vkms_connector_detect() take the
same configfs device lock (connector->dev->lock) that
connector_release() already holds while unlinking/freeing. That would
deadlock: vkms_destroy() (reached from device_enabled_store() while
connector->dev->lock is held) calls drm_dev_unregister() and
drm_atomic_helper_shutdown(), both of which take
drm_device.mode_config.mutex internally. That establishes lock order
configfs-lock -> mode_config.mutex on the teardown path, while
vkms_connector_detect() always runs already holding
mode_config.mutex -- so having it additionally acquire the configfs
lock would order it mode_config.mutex -> configfs-lock, the exact
reverse (ABBA).

Fix the race with RCU instead, decoupling the free side from the read
side without introducing a new lock-ordering constraint:

  - vkms_config_create_connector() links with list_add_tail_rcu()
    instead of list_add_tail().
  - vkms_config_destroy_connector() unlinks with list_del_rcu() instead
    of list_del(), and defers the actual free with kfree_rcu() instead
    of kfree(). xa_destroy() of connector_cfg->possible_encoders still
    runs synchronously, which is safe because no RCU reader (i.e.
    vkms_connector_detect()) ever touches that field.
  - A new vkms_config_for_each_connector_rcu() iterator
    (list_for_each_entry_rcu()) is added alongside the existing
    vkms_config_for_each_connector() for the other (non-concurrent,
    configfs-lock-serialized) call sites, and used only in
    vkms_connector_detect(), wrapped in rcu_read_lock()/
    rcu_read_unlock().

This is the same pattern DRM core already uses to let connector
iteration (drm_connector_list_iter) survive concurrent connector
teardown without holding mode_config.mutex across the whole walk;
here it is applied locally to vkms's own configfs connector list,
matching the existing "protect the config lists, use RCU when a
reader can't take the writer's lock" discipline already present in
the file.

No other vkms_config_for_each_connector() call site (vkms_output.c,
vkms_config.c is_valid()/debugfs helpers) races a concurrent free:
they all run either at device-build time (single-threaded) or already
under connector->dev->lock, so they are left untouched to keep the fix
minimal and scoped to the reported sink.

Verified on a v6.19 KASAN-instrumented build: a detect-hammer vs.
connector-rmdir stress loop reliably tripped a "KASAN: slab-use-after-
free in vkms_connector_detect" report before this patch; with the fix
applied, the same stress loop no longer produces any KASAN report.

Fixes: 466f43885ac0 ("drm/vkms: Allow to update the connector status")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 drivers/gpu/drm/vkms/vkms_config.c    | 24 +++++++++++++++++++++---
 drivers/gpu/drm/vkms/vkms_config.h    | 22 ++++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_connector.c | 16 +++++++++++++++-
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
index 5a654d6dead8..fc402cf48775 100644
--- a/drivers/gpu/drm/vkms/vkms_config.c
+++ b/drivers/gpu/drm/vkms/vkms_config.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 
+#include <linux/rculist.h>
+#include <linux/rcupdate.h>
 #include <linux/slab.h>
 
 #include <drm/drm_print.h>
@@ -599,7 +601,12 @@ struct vkms_config_connector *vkms_config_create_connector(struct vkms_config *c
 	connector_cfg->status = connector_status_connected;
 	xa_init_flags(&connector_cfg->possible_encoders, XA_FLAGS_ALLOC);
 
-	list_add_tail(&connector_cfg->link, &config->connectors);
+	/*
+	 * Paired with vkms_config_for_each_connector_rcu() in
+	 * vkms_connector_detect(), which may walk this list without holding
+	 * the configfs device lock.
+	 */
+	list_add_tail_rcu(&connector_cfg->link, &config->connectors);
 
 	return connector_cfg;
 }
@@ -608,8 +615,19 @@ EXPORT_SYMBOL_IF_KUNIT(vkms_config_create_connector);
 void vkms_config_destroy_connector(struct vkms_config_connector *connector_cfg)
 {
 	xa_destroy(&connector_cfg->possible_encoders);
-	list_del(&connector_cfg->link);
-	kfree(connector_cfg);
+
+	/*
+	 * connector_release() (vkms_configfs.c) can free a connector while
+	 * vkms_connector_detect() is concurrently walking the connectors list
+	 * under its own RCU read-side critical section (it cannot take the
+	 * configfs device lock: it already runs under
+	 * drm_device.mode_config.mutex, and vkms_destroy() takes the two
+	 * locks in the opposite order, so plain locking here would deadlock).
+	 * Unlink with the RCU-safe primitive and defer the free to after a
+	 * grace period so concurrent readers never dereference freed memory.
+	 */
+	list_del_rcu(&connector_cfg->link);
+	kfree_rcu(connector_cfg, rcu);
 }
 EXPORT_SYMBOL_IF_KUNIT(vkms_config_destroy_connector);
 
diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
index 8f7f286a4bdd..127076a9280b 100644
--- a/drivers/gpu/drm/vkms/vkms_config.h
+++ b/drivers/gpu/drm/vkms/vkms_config.h
@@ -4,6 +4,7 @@
 #define _VKMS_CONFIG_H_
 
 #include <linux/list.h>
+#include <linux/rculist.h>
 #include <linux/types.h>
 #include <linux/xarray.h>
 
@@ -108,6 +109,9 @@ struct vkms_config_encoder {
  *             It can be used to store a temporary reference to a VKMS connector
  *             during device creation. This pointer is not managed by the
  *             configuration and must be managed by other means.
+ * @rcu: Used to free the connector configuration after a grace period, so that
+ *       vkms_config_for_each_connector_rcu() readers (e.g. .detect) can safely
+ *       run concurrently with configfs teardown (see vkms_config_destroy_connector()).
  */
 struct vkms_config_connector {
 	struct list_head link;
@@ -118,6 +122,8 @@ struct vkms_config_connector {
 
 	/* Internal usage */
 	struct vkms_connector *connector;
+
+	struct rcu_head rcu;
 };
 
 /**
@@ -152,6 +158,22 @@ struct vkms_config_connector {
 #define vkms_config_for_each_connector(config, connector_cfg) \
 	list_for_each_entry((connector_cfg), &(config)->connectors, link)
 
+/**
+ * vkms_config_for_each_connector_rcu - RCU-safe iteration over the vkms_config
+ * connectors
+ * @config: &struct vkms_config pointer
+ * @connector_cfg: &struct vkms_config_connector pointer used as cursor
+ *
+ * Unlike vkms_config_for_each_connector(), this may be used without holding
+ * the configfs device lock, from contexts (such as &drm_connector_funcs.detect)
+ * that must not block on it. Callers must wrap the iteration in
+ * rcu_read_lock() / rcu_read_unlock(); see vkms_config_destroy_connector(),
+ * which pairs with this by unlinking with list_del_rcu() and freeing with
+ * kfree_rcu().
+ */
+#define vkms_config_for_each_connector_rcu(config, connector_cfg) \
+	list_for_each_entry_rcu((connector_cfg), &(config)->connectors, link)
+
 /**
  * vkms_config_plane_for_each_possible_crtc - Iterate over the vkms_config_plane
  * possible CRTCs
diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
index b0a6b212d3f4..c43948de2b01 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.c
+++ b/drivers/gpu/drm/vkms/vkms_connector.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 
+#include <linux/rcupdate.h>
+
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_managed.h>
@@ -26,10 +28,22 @@ static enum drm_connector_status vkms_connector_detect(struct drm_connector *con
 	 */
 	status = connector->status;
 
-	vkms_config_for_each_connector(vkmsdev->config, connector_cfg) {
+	/*
+	 * configfs can free connector_cfg concurrently (connector_release() ->
+	 * vkms_config_destroy_connector(), on an rmdir of the connector's
+	 * configfs directory) without taking drm_device.mode_config.mutex,
+	 * which this .detect callback is always called under. Walk the RCU-
+	 * protected list instead of taking the configfs device lock here: the
+	 * two locks are already nested in the opposite order by
+	 * vkms_destroy(), so acquiring the configfs lock from under
+	 * mode_config.mutex would deadlock.
+	 */
+	rcu_read_lock();
+	vkms_config_for_each_connector_rcu(vkmsdev->config, connector_cfg) {
 		if (connector_cfg->connector == vkms_connector)
 			status = vkms_config_connector_get_status(connector_cfg);
 	}
+	rcu_read_unlock();
 
 	return status;
 }
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] drm/vkms: Fix UAF between connector configfs rmdir and .detect

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] RCU-001 violation: `xa_destroy` is called on `possible_encoders` before the object is unlinked from the RCU list.

Pre-existing issues:
- [High] The debugfs helper `vkms_config_show` is vulnerable to a Use-After-Free because it traverses configuration lists without locks or RCU protection.
--

--- Patch [1]: [PATCH] drm/vkms: Fix UAF between connector configfs rmdir and .detect ---
commit 7286ff600c4e0777d618b609fd944dc368ecbf69
Author: Ibrahim Hashimov <security@auditcode.ai>

drm/vkms: Fix UAF between connector configfs rmdir and .detect

This commit addresses a race condition between concurrent configfs directory
removal and drm connector detection in the vkms driver. It introduces RCU
protection for the connector configuration list to decouple the free side
from the read side, avoiding a lock-ordering deadlock.

> No other vkms_config_for_each_connector() call site (vkms_output.c,
> vkms_config.c is_valid()/debugfs helpers) races a concurrent free:
> they all run either at device-build time (single-threaded) or already
> under connector->dev->lock, so they are left untouched to keep the fix
> minimal and scoped to the reported sink.

[Severity: High]
This is a pre-existing issue, but is this statement about debugfs helpers
accurate?

Looking at vkms_config_show() in drivers/gpu/drm/vkms/vkms_config.c, it is
invoked by the VFS seq_read and iterates over the device's planes, CRTCs,
encoders, and connectors locklessly. It does not appear to acquire the
configfs device mutex dev->lock.

Since configfs permits users to rmdir configuration directories at any time,
a concurrent rmdir triggers the configfs release callbacks like
plane_release() and connector_release(). For planes, CRTCs, and encoders,
these callbacks synchronously kfree() the objects.

Could a concurrent rmdir executed during a debugfs read cause
vkms_config_show() to traverse into freed memory?

> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index 5a654d6dead84..fc402cf487753 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
[ ... ]
> @@ -608,8 +615,19 @@ EXPORT_SYMBOL_IF_KUNIT(vkms_config_create_connector);
>  void vkms_config_destroy_connector(struct vkms_config_connector *connector_cfg)
>  {
>  	xa_destroy(&connector_cfg->possible_encoders);

[Severity: Medium]
Does this code fully unlink the object from the RCU-protected list before
destroying its components?

In vkms_config_destroy_connector(), xa_destroy() is called on
connector_cfg->possible_encoders before list_del_rcu() removes the object
from the list. This leaves the object partially destructed while it is still
globally visible to concurrent RCU readers traversing the config->connectors
list.

Even if the current RCU reader does not access this field, should the object
be unlinked via list_del_rcu() before calling xa_destroy() to ensure safe
remove-before-reclaim ordering?

> -	list_del(&connector_cfg->link);
> -	kfree(connector_cfg);
> +
> +	/*
> +	 * connector_release() (vkms_configfs.c) can free a connector while
> +	 * vkms_connector_detect() is concurrently walking the connectors list
> +	 * under its own RCU read-side critical section (it cannot take the
> +	 * configfs device lock: it already runs under
> +	 * drm_device.mode_config.mutex, and vkms_destroy() takes the two
> +	 * locks in the opposite order, so plain locking here would deadlock).
> +	 * Unlink with the RCU-safe primitive and defer the free to after a
> +	 * grace period so concurrent readers never dereference freed memory.
> +	 */
> +	list_del_rcu(&connector_cfg->link);
> +	kfree_rcu(connector_cfg, rcu);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709150637.45052-1-security@auditcode.ai?part=1

[PATCH v2] drm/vkms: Fix UAF between connector configfs rmdir and .detect

[Ibrahim Hashimov]

vkms_connector_detect() walks config->connectors via
vkms_config_for_each_connector(), which expands to a plain
list_for_each_entry() over vkms_config::connectors with no lock and no
RCU protection.

Concurrently, rmdir'ing a connector's configfs directory calls
connector_release(), which takes the configfs device mutex
(connector->dev->lock) and calls vkms_config_destroy_connector(),
which does:

	list_del(&connector_cfg->link);
	kfree(connector_cfg);

Unlike make_crtc_group()/make_plane_group()/make_encoder_group()/
make_connector_group() and the possible_crtcs/possible_encoders
allow_link() callbacks, all of which refuse the operation with -EBUSY
while the device is enabled, connector_release() has no such guard, and
in fact cannot be given one: release() runs after configfs has already
committed to removing the directory, so it cannot fail the rmdir(2)
syscall with -EBUSY. A connector's configfs directory can therefore be
removed, and its vkms_config_connector freed, at any time while the
device is live and its DRM connector is still being probed.

vkms_connector_detect() is reached from userspace by writing "detect"
to /sys/class/drm/<card>-<conn>/status (drm_sysfs status_store() ->
drm_helper_probe_single_connector_modes() -> .detect), which runs under
drm_device.mode_config.mutex. Nothing prevents this from running at the
same time as the configfs rmdir above, since the two paths take
entirely different locks (configfs connector->dev->lock vs. DRM's
mode_config.mutex) over the same vkms_config_connector object. Both
sides require local root/CAP_SYS_ADMIN: the vkms configfs tree is
root-owned (0755) and the connector's sysfs status attribute is
root-writable by default, so this is a local-root race, not an
unprivileged-user or remote issue.

The result is a classic race-into-use-after-free: the unlocked iterator
dereferences connector_cfg->connector or follows connector_cfg->link
into memory that connector_release() has already kfree()'d, or reads
connector_cfg->status out of freed memory. This reproduces as a KASAN
slab-use-after-free in vkms_connector_detect() under a detect-hammer
vs. connector-rmdir stress loop.

A tempting minimal fix is to have vkms_connector_detect() take the same
configfs device lock (connector->dev->lock) that connector_release()
already holds while unlinking/freeing. That would deadlock:
vkms_destroy() (reached from device_enabled_store() while
connector->dev->lock is held) calls drm_dev_unregister() and
drm_atomic_helper_shutdown(), both of which take
drm_device.mode_config.mutex internally. That establishes lock order
configfs-lock -> mode_config.mutex on the teardown path, while
vkms_connector_detect() always runs already holding mode_config.mutex,
so having it additionally acquire the configfs lock would order it
mode_config.mutex -> configfs-lock, the exact reverse (ABBA).

Fix the race with RCU instead, decoupling the free side from the read
side without introducing a new lock-ordering constraint:

  - vkms_config_create_connector() links with list_add_tail_rcu()
    instead of list_add_tail().
  - vkms_config_destroy_connector() unlinks with list_del_rcu() before
    it destroys the object, then defers the free with kfree_rcu()
    instead of kfree(). It unlinks first so the object is removed from
    the list before it is reclaimed; xa_destroy() of
    connector_cfg->possible_encoders then runs synchronously between the
    unlink and the deferred free, which is safe because no RCU reader
    (i.e. vkms_connector_detect()) ever dereferences that field.
  - A new vkms_config_for_each_connector_rcu() iterator
    (list_for_each_entry_rcu()) is added alongside the existing
    vkms_config_for_each_connector() for the other (non-concurrent,
    configfs-lock-serialized) call sites, and used only in
    vkms_connector_detect(), wrapped in rcu_read_lock()/
    rcu_read_unlock().

This is the same pattern DRM core already uses to let connector
iteration (drm_connector_list_iter) survive concurrent connector
teardown without holding mode_config.mutex across the whole walk; here
it is applied locally to vkms's own configfs connector list, matching
the existing "protect the config lists, use RCU when a reader can't
take the writer's lock" discipline already present in the file.

This patch is scoped to the .detect sink. vkms_config_show(), the read
handler for the "vkms_config" debugfs file, also walks the plane, CRTC,
encoder and connector config lists without any lock or RCU protection,
and is subject to a separate, pre-existing use-after-free against a
concurrent configfs rmdir. Fixing it properly requires RCU-converting
the plane, CRTC and encoder teardown paths too:
vkms_config_destroy_plane(), vkms_config_destroy_crtc() and
vkms_config_destroy_encoder() still free their objects non-RCU (plain
list_del() + kfree()), so the RCU conversion done here for connectors
alone is not enough to make that debugfs walk safe. That is left as a
separate follow-up and is not addressed here.

Verified on a v6.19 KASAN-instrumented build: a detect-hammer vs.
connector-rmdir stress loop reliably tripped a "KASAN: slab-use-after-
free in vkms_connector_detect" report before this patch; with the fix
applied, the same stress loop no longer produces any KASAN report.

Fixes: 466f43885ac0 ("drm/vkms: Allow to update the connector status")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: address sashiko-bot review of v1
(https://lore.kernel.org/dri-devel/20260709150637.45052-1-security@auditcode.ai/):
reorder vkms_config_destroy_connector() to unlink (list_del_rcu) before
destroying components (xa_destroy) for remove-before-reclaim ordering; drop the
v1 claim that vkms_config_show() is safe -- it is a separate pre-existing
lockless-traversal issue, noted as out of scope. No change to the RCU .detect fix.
 drivers/gpu/drm/vkms/vkms_config.c    | 28 ++++++++++++++++++++++++---
 drivers/gpu/drm/vkms/vkms_config.h    | 22 +++++++++++++++++++++
 drivers/gpu/drm/vkms/vkms_connector.c | 16 ++++++++++++++-
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
index 5a654d6dead8..a44e0567f8a3 100644
--- a/drivers/gpu/drm/vkms/vkms_config.c
+++ b/drivers/gpu/drm/vkms/vkms_config.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 
+#include <linux/rculist.h>
+#include <linux/rcupdate.h>
 #include <linux/slab.h>
 
 #include <drm/drm_print.h>
@@ -599,7 +601,12 @@ struct vkms_config_connector *vkms_config_create_connector(struct vkms_config *c
 	connector_cfg->status = connector_status_connected;
 	xa_init_flags(&connector_cfg->possible_encoders, XA_FLAGS_ALLOC);
 
-	list_add_tail(&connector_cfg->link, &config->connectors);
+	/*
+	 * Paired with vkms_config_for_each_connector_rcu() in
+	 * vkms_connector_detect(), which may walk this list without holding
+	 * the configfs device lock.
+	 */
+	list_add_tail_rcu(&connector_cfg->link, &config->connectors);
 
 	return connector_cfg;
 }
@@ -607,9 +614,24 @@ EXPORT_SYMBOL_IF_KUNIT(vkms_config_create_connector);
 
 void vkms_config_destroy_connector(struct vkms_config_connector *connector_cfg)
 {
+	/*
+	 * connector_release() (vkms_configfs.c) can free a connector while
+	 * vkms_connector_detect() is concurrently walking the connectors list
+	 * under its own RCU read-side critical section (it cannot take the
+	 * configfs device lock: it already runs under
+	 * drm_device.mode_config.mutex, and vkms_destroy() takes the two
+	 * locks in the opposite order, so plain locking here would deadlock).
+	 *
+	 * Unlink from the list before destroying the object's components, so
+	 * the object is removed before it is reclaimed. After list_del_rcu()
+	 * no new reader can reach it, and the only RCU reader
+	 * (vkms_connector_detect()) never dereferences possible_encoders, so
+	 * tearing that down next is safe. Defer the free to after a grace
+	 * period so concurrent readers never touch freed memory.
+	 */
+	list_del_rcu(&connector_cfg->link);
 	xa_destroy(&connector_cfg->possible_encoders);
-	list_del(&connector_cfg->link);
-	kfree(connector_cfg);
+	kfree_rcu(connector_cfg, rcu);
 }
 EXPORT_SYMBOL_IF_KUNIT(vkms_config_destroy_connector);
 
diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
index 8f7f286a4bdd..127076a9280b 100644
--- a/drivers/gpu/drm/vkms/vkms_config.h
+++ b/drivers/gpu/drm/vkms/vkms_config.h
@@ -4,6 +4,7 @@
 #define _VKMS_CONFIG_H_
 
 #include <linux/list.h>
+#include <linux/rculist.h>
 #include <linux/types.h>
 #include <linux/xarray.h>
 
@@ -108,6 +109,9 @@ struct vkms_config_encoder {
  *             It can be used to store a temporary reference to a VKMS connector
  *             during device creation. This pointer is not managed by the
  *             configuration and must be managed by other means.
+ * @rcu: Used to free the connector configuration after a grace period, so that
+ *       vkms_config_for_each_connector_rcu() readers (e.g. .detect) can safely
+ *       run concurrently with configfs teardown (see vkms_config_destroy_connector()).
  */
 struct vkms_config_connector {
 	struct list_head link;
@@ -118,6 +122,8 @@ struct vkms_config_connector {
 
 	/* Internal usage */
 	struct vkms_connector *connector;
+
+	struct rcu_head rcu;
 };
 
 /**
@@ -152,6 +158,22 @@ struct vkms_config_connector {
 #define vkms_config_for_each_connector(config, connector_cfg) \
 	list_for_each_entry((connector_cfg), &(config)->connectors, link)
 
+/**
+ * vkms_config_for_each_connector_rcu - RCU-safe iteration over the vkms_config
+ * connectors
+ * @config: &struct vkms_config pointer
+ * @connector_cfg: &struct vkms_config_connector pointer used as cursor
+ *
+ * Unlike vkms_config_for_each_connector(), this may be used without holding
+ * the configfs device lock, from contexts (such as &drm_connector_funcs.detect)
+ * that must not block on it. Callers must wrap the iteration in
+ * rcu_read_lock() / rcu_read_unlock(); see vkms_config_destroy_connector(),
+ * which pairs with this by unlinking with list_del_rcu() and freeing with
+ * kfree_rcu().
+ */
+#define vkms_config_for_each_connector_rcu(config, connector_cfg) \
+	list_for_each_entry_rcu((connector_cfg), &(config)->connectors, link)
+
 /**
  * vkms_config_plane_for_each_possible_crtc - Iterate over the vkms_config_plane
  * possible CRTCs
diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
index b0a6b212d3f4..c43948de2b01 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.c
+++ b/drivers/gpu/drm/vkms/vkms_connector.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 
+#include <linux/rcupdate.h>
+
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_managed.h>
@@ -26,10 +28,22 @@ static enum drm_connector_status vkms_connector_detect(struct drm_connector *con
 	 */
 	status = connector->status;
 
-	vkms_config_for_each_connector(vkmsdev->config, connector_cfg) {
+	/*
+	 * configfs can free connector_cfg concurrently (connector_release() ->
+	 * vkms_config_destroy_connector(), on an rmdir of the connector's
+	 * configfs directory) without taking drm_device.mode_config.mutex,
+	 * which this .detect callback is always called under. Walk the RCU-
+	 * protected list instead of taking the configfs device lock here: the
+	 * two locks are already nested in the opposite order by
+	 * vkms_destroy(), so acquiring the configfs lock from under
+	 * mode_config.mutex would deadlock.
+	 */
+	rcu_read_lock();
+	vkms_config_for_each_connector_rcu(vkmsdev->config, connector_cfg) {
 		if (connector_cfg->connector == vkms_connector)
 			status = vkms_config_connector_get_status(connector_cfg);
 	}
+	rcu_read_unlock();
 
 	return status;
 }
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] drm/vkms: Fix UAF between connector configfs rmdir and .detect

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unprotected list traversal in vkms_config_show() debugfs handler causes Use-After-Free.
- [High] Missing DRM device unplug synchronization allows Use-After-Free on vkmsdev->config during device teardown.
--

--- Patch [1]: [PATCH v2] drm/vkms: Fix UAF between connector configfs rmdir and .detect ---
commit 0675814c013a710595cbeaa530445c1e4fc38596
Author: Ibrahim Hashimov <security@auditcode.ai>

drm/vkms: Fix UAF between connector configfs rmdir and .detect

vkms_connector_detect() walks config->connectors via
vkms_config_for_each_connector(), which expands to a plain
list_for_each_entry() over vkms_config::connectors with no lock and no
RCU protection.

[ ... ]

> This patch is scoped to the .detect sink. vkms_config_show(), the read
> handler for the "vkms_config" debugfs file, also walks the plane, CRTC,
> encoder and connector config lists without any lock or RCU protection,
> and is subject to a separate, pre-existing use-after-free against a
> concurrent configfs rmdir. Fixing it properly requires RCU-converting
> the plane, CRTC and encoder teardown paths too:
> vkms_config_destroy_plane(), vkms_config_destroy_crtc() and
> vkms_config_destroy_encoder() still free their objects non-RCU (plain
> list_del() + kfree()), so the RCU conversion done here for connectors
> alone is not enough to make that debugfs walk safe. That is left as a
> separate follow-up and is not addressed here.

[Severity: High]
This is a pre-existing issue, but since vkms_config_show() in
drivers/gpu/drm/vkms/vkms_config.c uses unprotected list traversal, could
this cause a use-after-free if it iterates over planes or crtcs while they
are concurrently removed?

Reading the vkms_config debugfs file calls vkms_config_show(), which
iterates over planes, crtcs, and encoders using unlocked list traversals
like vkms_config_for_each_plane(). Concurrently, removing a plane via
configfs rmdir triggers plane_release(), which calls
vkms_config_destroy_plane() to immediately unlink and free the plane
configuration with kfree().

I understand you explicitly noted this as out of scope for the current
patch, but wanted to verify if it will be tracked separately.

> Verified on a v6.19 KASAN-instrumented build: a detect-hammer vs.
> connector-rmdir stress loop reliably tripped a "KASAN: slab-use-after-
> free in vkms_connector_detect" report before this patch; with the fix
> applied, the same stress loop no longer produces any KASAN report.
> 
> Fixes: 466f43885ac0 ("drm/vkms: Allow to update the connector status")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07

> diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
> index b0a6b212d3f4d..c43948de2b017 100644
> --- a/drivers/gpu/drm/vkms/vkms_connector.c
> +++ b/drivers/gpu/drm/vkms/vkms_connector.c

[ ... ]

> @@ -26,10 +28,22 @@ static enum drm_connector_status vkms_connector_detect(struct drm_connector *con
>  	 */
>  	status = connector->status;
>  
> -	vkms_config_for_each_connector(vkmsdev->config, connector_cfg) {
> +	/*
> +	 * configfs can free connector_cfg concurrently (connector_release() ->
> +	 * vkms_config_destroy_connector(), on an rmdir of the connector's
> +	 * configfs directory) without taking drm_device.mode_config.mutex,
> +	 * which this .detect callback is always called under. Walk the RCU-
> +	 * protected list instead of taking the configfs device lock here: the
> +	 * two locks are already nested in the opposite order by
> +	 * vkms_destroy(), so acquiring the configfs lock from under
> +	 * mode_config.mutex would deadlock.
> +	 */
> +	rcu_read_lock();
> +	vkms_config_for_each_connector_rcu(vkmsdev->config, connector_cfg) {

[Severity: High]
This isn't a bug introduced by this patch, but does the lack of DRM device
unplug synchronization here allow a use-after-free on vkmsdev->config?

If a user-space process calls a DRM ioctl like DRM_IOCTL_MODE_GETCONNECTOR,
it enters vkms_connector_detect() and accesses vkmsdev->config. Concurrently,
if a user deletes the device directory in configfs, device_release() is
triggered:

drivers/gpu/drm/vkms/vkms_configfs.c:device_release() {
    ...
    vkms_destroy(dev->config);
    kfree(dev->config);
}

Because vkms_connector_detect() does not protect its execution with
drm_dev_enter() (and vkms_destroy() uses drm_dev_unregister() instead of
drm_dev_unplug()), the device teardown does not wait for in-flight ioctls
to finish.

The drm_device is kept alive by the open file descriptor, but dev->config
is freed, which could cause the ioctl to dereference a dangling pointer.

>  		if (connector_cfg->connector == vkms_connector)
>  			status = vkms_config_connector_get_status(connector_cfg);
>  	}
> +	rcu_read_unlock();
>  
>  	return status;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709195129.50856-1-security@auditcode.ai?part=1