public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] Pass down hot-plug CONNECTOR ID to user-space
@ 2026-01-23 19:44 Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 1/4] drm: Introduce pending_hp to drm_connector Nicolas Frattaroli
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Nicolas Frattaroli @ 2026-01-23 19:44 UTC (permalink / raw)
  To: Ville Syrjälä, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Louis Chauvet,
	Haneen Mohammed, Melissa Wen, Daniel Stone, Ian Forbes,
	Dmitry Baryshkov
  Cc: dri-devel, linux-kernel, kernel, Nicolas Frattaroli, Marius Vlad

I will be taking over this series from Marius Vlad. 

This series addresses a shortcoming whereby a hot plug event is sent
without it being passed the actual connector that caused it. This takes
into consideration both the polling path and the HPD (Hot Plug Detect)
path. It also adds support for the vkms driver (using ConfigFS) for
propagating the connector ID when changing the connector's status.

The motivation is that user-space applications such as Weston would
previously receive non-connector-specific hotplug events, and then have
to figure out themselves which connector needs to have a modeset
executed on. This notably did not work when the hotplug events came in
too fast, resulting in Weston missing an on-off-on transition of a
connector, seeing that its state was unchanged from "on" so can't be the
one that was hotplugged, and skipping reinitialising it as it looks
through the other connectors that could've caused it.

The real world implication is that on setups with slightly sketchy HDMI
connections, a brief flicker in the HPD signal could result in video
output bidding farewell entirely until a manual proper re-plug was
performed.

By sending connector specific hotplug events, this ambiguity is
resolved without any change to the user-space API.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v6:
- Rewrote cover letter to explain the motivation for this series more
  plainly
- Rename "status_changed" to "pending_hp"
- Set "pending_hp" in the existing path that would also affect
  epoch_counter
- No longer set the boolean in drm_helper_probe_single_connector_modes,
  as it does not appear to be necessary
- Reword commits to better justify the changes
- Link to v5: https://lore.kernel.org/r/20251111162338.15141-1-marius.vlad@collabora.com/

Changes in v5:
- vkms: add support for sending the CONNECTOR ID when hot-plugging through
  ConfigFS - as reported by Louis, vkms can now make use of ConfigFS to
  simulate connector status.
- vkms: add a small change to ignore previous/old drm connector status
  when sending out hot-plug uevent.
- Link to v4: https://lore.kernel.org/r/20251103174558.7709-1-marius.vlad@collabora.com/

Changes in v4:
- removed the "This patch" bit - Dmitry
- added a short note when the flag is set and cleared - Dmitry
- address double dead-locking detected - kbot: https://lore.kernel.org/dri-devel/202509251410.fdfbcac3-lkp@intel.com/
- virtual connectors do not seem have any kind of hotplug - added
  polling in vkms - as noted by Ian
- Link to v3: https://lore.kernel.org/r/20250923083636.4749-1-marius.vlad@collabora.com/

Changes in v3:
- Address comments from Dmitry:
  - guard connector status write with mode_config.mutex
  - avoid setting up the connector status and immediately unset it. Do the
    unset in drm_kms_helper_hotplug_event/drm_kms_helper_connector_hotplug_event
- Link to v2: https://lore.kernel.org/r/20250729165708.9947-1-marius.vlad@collabora.com/

Changes in v2:
- Address comments from Daniel:
  - split patch into 2, one that introduces a bool to track connector
    connection status change and a patch that uses that to be able to send
    hot plug events with the proper CONNECTOR ID to udev and further pass
    that down to user-space
  - nuke out mutex when iterating connector list
  - fix typo
- Link to v1: https://lore.kernel.org/r/20250627131751.2004-1-marius.vlad@collabora.com/

Marius Vlad (4):
  drm: Introduce a new connector status
  drm: Propagate connector status change
  vkms: Do not send hotplug events for same connector status
  vkms: Pass the vkms connector as opposed to the vkms device

 drivers/gpu/drm/drm_connector.c       |  1 +
 drivers/gpu/drm/drm_probe_helper.c    | 39 +++++++++++++++++++++++----
 drivers/gpu/drm/drm_sysfs.c           |  1 +
 drivers/gpu/drm/vkms/vkms_configfs.c  | 12 +++++++--
 drivers/gpu/drm/vkms/vkms_connector.c |  6 ++---
 drivers/gpu/drm/vkms/vkms_connector.h |  4 +--
 include/drm/drm_connector.h           |  3 +++
 7 files changed, 54 insertions(+), 12 deletions(-)

--
2.47.2

---
Marius Vlad (4):
      drm: Introduce pending_hp to drm_connector
      drm: Send per-connector hotplug events
      vkms: Do not send hotplug events for same connector status
      vkms: Pass the vkms connector as opposed to the device on hotplug

 drivers/gpu/drm/drm_connector.c       |  1 +
 drivers/gpu/drm/drm_probe_helper.c    | 39 ++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/drm_sysfs.c           |  2 ++
 drivers/gpu/drm/vkms/vkms_configfs.c  |  6 ++++--
 drivers/gpu/drm/vkms/vkms_connector.c |  6 +++---
 drivers/gpu/drm/vkms/vkms_connector.h |  4 ++--
 include/drm/drm_connector.h           |  3 +++
 7 files changed, 49 insertions(+), 12 deletions(-)
---
base-commit: ab42b1c3fe4ce1ae5534c51b880d3a97e6bba145
change-id: 20260121-hot-plug-passup-f8ed03f7c202

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>


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

* [PATCH v6 1/4] drm: Introduce pending_hp to drm_connector
  2026-01-23 19:44 [PATCH v6 0/4] Pass down hot-plug CONNECTOR ID to user-space Nicolas Frattaroli
@ 2026-01-23 19:44 ` Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 2/4] drm: Send per-connector hotplug events Nicolas Frattaroli
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Nicolas Frattaroli @ 2026-01-23 19:44 UTC (permalink / raw)
  To: Ville Syrjälä, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Louis Chauvet,
	Haneen Mohammed, Melissa Wen, Daniel Stone, Ian Forbes,
	Dmitry Baryshkov
  Cc: dri-devel, linux-kernel, kernel, Nicolas Frattaroli, Marius Vlad

From: Marius Vlad <marius.vlad@collabora.com>

Introduce a new boolean variable used to track whether a connector has
changed its status since the last hotplug event for it was sent to
userspace. It is used by both the polling and HPD path.

A subsequent change will make use of this new member to propagate
per-connector udev hotplug events to userspace, rather than sending a
global hotplug event.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_connector.c    |  1 +
 drivers/gpu/drm/drm_probe_helper.c | 17 +++++++++++++++++
 drivers/gpu/drm/drm_sysfs.c        |  2 ++
 include/drm/drm_connector.h        |  3 +++
 4 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4f5b27fab475..aa8a187578da 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -274,6 +274,7 @@ static int drm_connector_init_only(struct drm_device *dev,
 
 	/* provide ddc symlink in sysfs */
 	connector->ddc = ddc;
+	connector->pending_hp = false;
 
 	INIT_LIST_HEAD(&connector->head);
 	INIT_LIST_HEAD(&connector->global_connector_list_entry);
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 09b12c30df69..91de34a25bf8 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -732,6 +732,17 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  */
 void drm_kms_helper_hotplug_event(struct drm_device *dev)
 {
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		connector->pending_hp = false;
+	}
+	drm_connector_list_iter_end(&conn_iter);
+	mutex_unlock(&dev->mode_config.mutex);
+
 	drm_sysfs_hotplug_event(dev);
 	drm_client_dev_hotplug(dev);
 }
@@ -748,6 +759,10 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
 
+	mutex_lock(&dev->mode_config.mutex);
+	connector->pending_hp = false;
+	mutex_unlock(&dev->mode_config.mutex);
+
 	drm_sysfs_connector_hotplug_event(connector);
 	drm_client_dev_hotplug(dev);
 }
@@ -837,6 +852,7 @@ static void output_poll_execute(struct work_struct *work)
 				    old_epoch_counter, connector->epoch_counter);
 
 			changed = true;
+			connector->pending_hp = true;
 		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
@@ -1101,6 +1117,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 				first_changed_connector = connector;
 			}
 
+			connector->pending_hp = true;
 			changed++;
 		}
 	}
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index b01ffa4d6509..53076c2afd12 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -216,6 +216,8 @@ static ssize_t status_store(struct device *device,
 			    connector->base.id, connector->name,
 			    old_force, connector->force);
 
+		connector->pending_hp = true;
+
 		connector->funcs->fill_modes(connector,
 					     dev->mode_config.max_width,
 					     dev->mode_config.max_height);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 7eaec37ae1c7..84968b2d967a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2191,6 +2191,9 @@ struct drm_connector {
 	/** @force: a DRM_FORCE_<foo> state for forced mode sets */
 	enum drm_connector_force force;
 
+	/** @pending_hp: true if connector changed since last hotplug event */
+	bool pending_hp;
+
 	/**
 	 * @edid_override: Override EDID set via debugfs.
 	 *

-- 
2.52.0


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

* [PATCH v6 2/4] drm: Send per-connector hotplug events
  2026-01-23 19:44 [PATCH v6 0/4] Pass down hot-plug CONNECTOR ID to user-space Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 1/4] drm: Introduce pending_hp to drm_connector Nicolas Frattaroli
@ 2026-01-23 19:44 ` Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 3/4] vkms: Do not send hotplug events for same connector status Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug Nicolas Frattaroli
  3 siblings, 0 replies; 7+ messages in thread
From: Nicolas Frattaroli @ 2026-01-23 19:44 UTC (permalink / raw)
  To: Ville Syrjälä, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Louis Chauvet,
	Haneen Mohammed, Melissa Wen, Daniel Stone, Ian Forbes,
	Dmitry Baryshkov
  Cc: dri-devel, linux-kernel, kernel, Nicolas Frattaroli, Marius Vlad

From: Marius Vlad <marius.vlad@collabora.com>

Use the new pending_hp member of drm_connector to always send
per-connector hotplug events for those connectors that need it, rather
than sending a global event, or only an event for one connector.

On the HPD (Hot Plug Detect) path this change notifies all connectors,
rather than just first changed connector.

The polling path is changed to no longer send a connector-less hotplug
event, but similarly send a hotplug event for each changed connector.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 91de34a25bf8..b60b1ef39748 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -860,8 +860,14 @@ static void output_poll_execute(struct work_struct *work)
 	mutex_unlock(&dev->mode_config.mutex);
 
 out:
-	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+	if (changed) {
+		drm_connector_list_iter_begin(dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
+			if (connector->pending_hp)
+				drm_kms_helper_connector_hotplug_event(connector);
+		}
+		drm_connector_list_iter_end(&conn_iter);
+	}
 
 	if (repoll)
 		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
@@ -1124,10 +1130,16 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 	drm_connector_list_iter_end(&conn_iter);
 	mutex_unlock(&dev->mode_config.mutex);
 
-	if (changed == 1)
+	if (changed == 1) {
 		drm_kms_helper_connector_hotplug_event(first_changed_connector);
-	else if (changed > 0)
-		drm_kms_helper_hotplug_event(dev);
+	} else if (changed > 0) {
+		drm_connector_list_iter_begin(dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
+			if (connector->pending_hp)
+				drm_kms_helper_connector_hotplug_event(connector);
+		}
+		drm_connector_list_iter_end(&conn_iter);
+	}
 
 	if (first_changed_connector)
 		drm_connector_put(first_changed_connector);

-- 
2.52.0


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

* [PATCH v6 3/4] vkms: Do not send hotplug events for same connector status
  2026-01-23 19:44 [PATCH v6 0/4] Pass down hot-plug CONNECTOR ID to user-space Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 1/4] drm: Introduce pending_hp to drm_connector Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 2/4] drm: Send per-connector hotplug events Nicolas Frattaroli
@ 2026-01-23 19:44 ` Nicolas Frattaroli
  2026-01-23 19:44 ` [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug Nicolas Frattaroli
  3 siblings, 0 replies; 7+ messages in thread
From: Nicolas Frattaroli @ 2026-01-23 19:44 UTC (permalink / raw)
  To: Ville Syrjälä, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Louis Chauvet,
	Haneen Mohammed, Melissa Wen, Daniel Stone, Ian Forbes,
	Dmitry Baryshkov
  Cc: dri-devel, linux-kernel, kernel, Nicolas Frattaroli, Marius Vlad

From: Marius Vlad <marius.vlad@collabora.com>

Only send a new hotplug event when writing to the connector status
configfs entry if the connector status changed compared to its
previous value.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/vkms/vkms_configfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index 506666e21c91..d6e203d59b45 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -549,9 +549,11 @@ static ssize_t connector_status_store(struct config_item *item,
 		return -EINVAL;
 
 	scoped_guard(mutex, &connector->dev->lock) {
+		enum drm_connector_status old_status =
+			vkms_config_connector_get_status(connector->config);
 		vkms_config_connector_set_status(connector->config, status);
 
-		if (connector->dev->enabled)
+		if (connector->dev->enabled && old_status != status)
 			vkms_trigger_connector_hotplug(connector->dev->config->dev);
 	}
 

-- 
2.52.0


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

* [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug
  2026-01-23 19:44 [PATCH v6 0/4] Pass down hot-plug CONNECTOR ID to user-space Nicolas Frattaroli
                   ` (2 preceding siblings ...)
  2026-01-23 19:44 ` [PATCH v6 3/4] vkms: Do not send hotplug events for same connector status Nicolas Frattaroli
@ 2026-01-23 19:44 ` Nicolas Frattaroli
  2026-01-24  1:06   ` Louis Chauvet
  3 siblings, 1 reply; 7+ messages in thread
From: Nicolas Frattaroli @ 2026-01-23 19:44 UTC (permalink / raw)
  To: Ville Syrjälä, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Louis Chauvet,
	Haneen Mohammed, Melissa Wen, Daniel Stone, Ian Forbes,
	Dmitry Baryshkov
  Cc: dri-devel, linux-kernel, kernel, Nicolas Frattaroli, Marius Vlad

From: Marius Vlad <marius.vlad@collabora.com>

By passing the connector rather than the device to
vkms_trigger_connector_hotplug, vkms can trigger connector hotplugging
events that contain the connector ID.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/vkms/vkms_configfs.c  | 2 +-
 drivers/gpu/drm/vkms/vkms_connector.c | 6 +++---
 drivers/gpu/drm/vkms/vkms_connector.h | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index d6e203d59b45..63a27f671e6a 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -554,7 +554,7 @@ static ssize_t connector_status_store(struct config_item *item,
 		vkms_config_connector_set_status(connector->config, status);
 
 		if (connector->dev->enabled && old_status != status)
-			vkms_trigger_connector_hotplug(connector->dev->config->dev);
+			vkms_trigger_connector_hotplug(connector->config->connector);
 	}
 
 	return (ssize_t)count;
diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
index b0a6b212d3f4..cad64eff72ea 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.c
+++ b/drivers/gpu/drm/vkms/vkms_connector.c
@@ -88,9 +88,9 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev)
 	return connector;
 }
 
-void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev)
+void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector)
 {
-	struct drm_device *dev = &vkmsdev->drm;
+	struct drm_connector *connector = &vkms_connector->base;
 
-	drm_kms_helper_hotplug_event(dev);
+	drm_kms_helper_connector_hotplug_event(connector);
 }
diff --git a/drivers/gpu/drm/vkms/vkms_connector.h b/drivers/gpu/drm/vkms/vkms_connector.h
index ed312f4eff3a..7cd76d01b10b 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.h
+++ b/drivers/gpu/drm/vkms/vkms_connector.h
@@ -28,8 +28,8 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev);
 
 /**
  * vkms_trigger_connector_hotplug() - Update the device's connectors status
- * @vkmsdev: VKMS device to update
+ * @vkms_connector: VKMS connector to update
  */
-void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev);
+void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector);
 
 #endif /* _VKMS_CONNECTOR_H_ */

-- 
2.52.0


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

* Re: [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug
  2026-01-23 19:44 ` [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug Nicolas Frattaroli
@ 2026-01-24  1:06   ` Louis Chauvet
  2026-02-09 19:38     ` Nicolas Frattaroli
  0 siblings, 1 reply; 7+ messages in thread
From: Louis Chauvet @ 2026-01-24  1:06 UTC (permalink / raw)
  To: Nicolas Frattaroli, Ville Syrjälä, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Haneen Mohammed, Melissa Wen, Daniel Stone, Ian Forbes,
	Dmitry Baryshkov
  Cc: dri-devel, linux-kernel, kernel, Marius Vlad

Hello Nicolas,

On the principle I agree with this patch, I just need to take time to 
properly think about lifetime of vkms_config vs connector and pointer 
validity to avoid use-after-free / null pointer dereference. I will try 
to review it next week or more probably just after the FOSDEM.

In the meantime, if you want to try / think about the possible issue: I 
think there will be a use-after-free if you unbind the driver using the 
sysfs interface and interract with configfs interface.

Thanks a lot for this series,
Louis Chauvet


On 1/23/26 20:44, Nicolas Frattaroli wrote:
> From: Marius Vlad <marius.vlad@collabora.com>
> 
> By passing the connector rather than the device to
> vkms_trigger_connector_hotplug, vkms can trigger connector hotplugging
> events that contain the connector ID.
> 
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>   drivers/gpu/drm/vkms/vkms_configfs.c  | 2 +-
>   drivers/gpu/drm/vkms/vkms_connector.c | 6 +++---
>   drivers/gpu/drm/vkms/vkms_connector.h | 4 ++--
>   3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index d6e203d59b45..63a27f671e6a 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -554,7 +554,7 @@ static ssize_t connector_status_store(struct config_item *item,
>   		vkms_config_connector_set_status(connector->config, status);
>   
>   		if (connector->dev->enabled && old_status != status)
> -			vkms_trigger_connector_hotplug(connector->dev->config->dev);
> +			vkms_trigger_connector_hotplug(connector->config->connector);

Here connector->config is valid, but connector->config->connector is 
probably invalid if the driver is unbind. We may need to add some 
refcount to avoid this kind of issue.

The other way around, I think there could be issue if the configfs 
folder is completly removed (that possible, there is no way to forbid 
deletions in configfs), the config object is freed but maybe used in the 
"DRM" part of VKMS (for connector status update maybe).

>   	}
>   
>   	return (ssize_t)count;
> diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
> index b0a6b212d3f4..cad64eff72ea 100644
> --- a/drivers/gpu/drm/vkms/vkms_connector.c
> +++ b/drivers/gpu/drm/vkms/vkms_connector.c
> @@ -88,9 +88,9 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev)
>   	return connector;
>   }
>   
> -void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev)
> +void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector)
>   {
> -	struct drm_device *dev = &vkmsdev->drm;
> +	struct drm_connector *connector = &vkms_connector->base;
>   
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_connector_hotplug_event(connector);
>   }
> diff --git a/drivers/gpu/drm/vkms/vkms_connector.h b/drivers/gpu/drm/vkms/vkms_connector.h
> index ed312f4eff3a..7cd76d01b10b 100644
> --- a/drivers/gpu/drm/vkms/vkms_connector.h
> +++ b/drivers/gpu/drm/vkms/vkms_connector.h
> @@ -28,8 +28,8 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev);
>   
>   /**
>    * vkms_trigger_connector_hotplug() - Update the device's connectors status
> - * @vkmsdev: VKMS device to update
> + * @vkms_connector: VKMS connector to update
>    */
> -void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev);
> +void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector);
>   
>   #endif /* _VKMS_CONNECTOR_H_ */
> 


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

* Re: [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug
  2026-01-24  1:06   ` Louis Chauvet
@ 2026-02-09 19:38     ` Nicolas Frattaroli
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Frattaroli @ 2026-02-09 19:38 UTC (permalink / raw)
  To: Ville Syrjälä, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Haneen Mohammed,
	Melissa Wen, Daniel Stone, Ian Forbes, Dmitry Baryshkov,
	Louis Chauvet
  Cc: dri-devel, linux-kernel, kernel, Marius Vlad

Hi Louis,

On Saturday, 24 January 2026 02:06:27 Central European Standard Time Louis Chauvet wrote:
> Hello Nicolas,
> 
> On the principle I agree with this patch, I just need to take time to 
> properly think about lifetime of vkms_config vs connector and pointer 
> validity to avoid use-after-free / null pointer dereference. I will try 
> to review it next week or more probably just after the FOSDEM.

I should've offered we could've discussed it in person at FOSDEM since
I was there as well, but I forgot. :(

> 
> In the meantime, if you want to try / think about the possible issue: I 
> think there will be a use-after-free if you unbind the driver using the 
> sysfs interface and interract with configfs interface.

I can sort of see what I think you're seeing, but the specific concern
below looks to me like something that already is a potential problem
in vkms. That is, if I understood the scenario envisioned here.

> 
> Thanks a lot for this series,
> Louis Chauvet
> 
> 
> On 1/23/26 20:44, Nicolas Frattaroli wrote:
> > From: Marius Vlad <marius.vlad@collabora.com>
> > 
> > By passing the connector rather than the device to
> > vkms_trigger_connector_hotplug, vkms can trigger connector hotplugging
> > events that contain the connector ID.
> > 
> > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >   drivers/gpu/drm/vkms/vkms_configfs.c  | 2 +-
> >   drivers/gpu/drm/vkms/vkms_connector.c | 6 +++---
> >   drivers/gpu/drm/vkms/vkms_connector.h | 4 ++--
> >   3 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> > index d6e203d59b45..63a27f671e6a 100644
> > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> > @@ -554,7 +554,7 @@ static ssize_t connector_status_store(struct config_item *item,
> >   		vkms_config_connector_set_status(connector->config, status);
> >   
> >   		if (connector->dev->enabled && old_status != status)
> > -			vkms_trigger_connector_hotplug(connector->dev->config->dev);
> > +			vkms_trigger_connector_hotplug(connector->config->connector);
> 
> Here connector->config is valid, but connector->config->connector is 
> probably invalid if the driver is unbind. We may need to add some 
> refcount to avoid this kind of issue.

I'm fairly sure whatever race condition you're worried about here
already exists in the old code anyway. connector->dev->config will
have its vkms_device first released, and then the config->dev member
set to NULL, without a lock, in vkms_destroy. That vkms_device is
allocated in vkms_create with devm_drm_dev_alloc using the fauxdev's
dev as a devres group, which is the one being freed in vkms_destroy
while momentarily leaving a dangling pointer in the config struct.

There's nothing fundamentally more broken here as this already is
if the configfs can somehow persist during vkms unbind, if I
understand you correctly by what you mean with driver unbind. The
vkms_configfs_unregister() call in vkms_exit before the vkms teardown
makes me think that this isn't supposed to happen anyway; on vkms
module exit, the configfs stuff is torn down before the vkms
resources are freed.

If it is a realistic scenario and I just disclosed a potential
use-after-free that's already in the kernel on a public mailing list,
then: oops, guess this one's a freebie for the red teams.

> The other way around, I think there could be issue if the configfs 
> folder is completly removed (that possible, there is no way to forbid 
> deletions in configfs), the config object is freed but maybe used in the 
> "DRM" part of VKMS (for connector status update maybe).

Is there ever a situation where a non-root user can delete configfs that
wouldn't also be a whole can of worms in other aspects?

A more general point:

The vkms part here was only ever there so the HPD stuff can be tested
without a hardware test setup or by poking the sysfs of real hardware.
I inherited it when I took the series over, but don't use it myself
beyond testing these two vkms patches.

If I need to add some refcounting scheme to vkms connectors then I
assume this will take a few more revisions to get right. I hope the
non-vkms parts can be reviewed by those who raised concerns on the
previous revision in the meantime, so I know whether I'm making
progress here.

I'll discuss with my colleagues whether the vkms parts are something
we absolutely need. If not, I'll just drop them in the next revision,
unless there's a fairly good case to be made from your side that
they'd find use and are worth the time investment.

Kind regards,
Nicolas Frattaroli

> 
> >   	}
> >   
> >   	return (ssize_t)count;
> > diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
> > index b0a6b212d3f4..cad64eff72ea 100644
> > --- a/drivers/gpu/drm/vkms/vkms_connector.c
> > +++ b/drivers/gpu/drm/vkms/vkms_connector.c
> > @@ -88,9 +88,9 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev)
> >   	return connector;
> >   }
> >   
> > -void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev)
> > +void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector)
> >   {
> > -	struct drm_device *dev = &vkmsdev->drm;
> > +	struct drm_connector *connector = &vkms_connector->base;
> >   
> > -	drm_kms_helper_hotplug_event(dev);
> > +	drm_kms_helper_connector_hotplug_event(connector);
> >   }
> > diff --git a/drivers/gpu/drm/vkms/vkms_connector.h b/drivers/gpu/drm/vkms/vkms_connector.h
> > index ed312f4eff3a..7cd76d01b10b 100644
> > --- a/drivers/gpu/drm/vkms/vkms_connector.h
> > +++ b/drivers/gpu/drm/vkms/vkms_connector.h
> > @@ -28,8 +28,8 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev);
> >   
> >   /**
> >    * vkms_trigger_connector_hotplug() - Update the device's connectors status
> > - * @vkmsdev: VKMS device to update
> > + * @vkms_connector: VKMS connector to update
> >    */
> > -void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev);
> > +void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector);
> >   
> >   #endif /* _VKMS_CONNECTOR_H_ */
> > 
> 
> 





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

end of thread, other threads:[~2026-02-09 19:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-23 19:44 [PATCH v6 0/4] Pass down hot-plug CONNECTOR ID to user-space Nicolas Frattaroli
2026-01-23 19:44 ` [PATCH v6 1/4] drm: Introduce pending_hp to drm_connector Nicolas Frattaroli
2026-01-23 19:44 ` [PATCH v6 2/4] drm: Send per-connector hotplug events Nicolas Frattaroli
2026-01-23 19:44 ` [PATCH v6 3/4] vkms: Do not send hotplug events for same connector status Nicolas Frattaroli
2026-01-23 19:44 ` [PATCH v6 4/4] vkms: Pass the vkms connector as opposed to the device on hotplug Nicolas Frattaroli
2026-01-24  1:06   ` Louis Chauvet
2026-02-09 19:38     ` Nicolas Frattaroli

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