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

[sashiko-bot@kernel.org]

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