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

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

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