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 <[email protected]> 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: [email protected] > Signed-off-by: Ibrahim Hashimov <[email protected]> > 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/[email protected]?part=1
