[PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup

Linux kernel -stable discussions
 help / color / mirror / Atom feed

* [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup
       [not found] <20221017153150.60675-1-contact@emersion.fr>
@ 2022-10-17 15:32 ` Simon Ser
  2022-10-17 15:34   ` Jonas Ådahl
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Simon Ser @ 2022-10-17 15:32 UTC (permalink / raw)
  To: dri-devel; +Cc: stable, Daniel Vetter, Lyude Paul, Jonas Ådahl

A typical DP-MST unplug removes a KMS connector. However care must
be taken to properly synchronize with user-space. The expected
sequence of events is the following:

1. The kernel notices that the DP-MST port is gone.
2. The kernel marks the connector as disconnected, then sends a
   uevent to make user-space re-scan the connector list.
3. User-space notices the connector goes from connected to disconnected,
   disables it.
4. Kernel handles the the IOCTL disabling the connector. On success,
   the very last reference to the struct drm_connector is dropped and
   drm_connector_cleanup() is called.
5. The connector is removed from the list, and a uevent is sent to tell
   user-space that the connector disappeared.

The very last step was missing. As a result, user-space thought the
connector still existed and could try to disable it again. Since the
kernel no longer knows about the connector, that would end up with
EINVAL and confused user-space.

Fix this by sending a hotplug uevent from drm_connector_cleanup().

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: stable@vger.kernel.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Jonas Ådahl <jadahl@redhat.com>
---
 drivers/gpu/drm/drm_connector.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index e3142c8142b3..90dad87e9ad0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -582,6 +582,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
 	mutex_destroy(&connector->mutex);

 	memset(connector, 0, sizeof(*connector));
+
+	if (dev->registered)
+		drm_sysfs_hotplug_event(dev);
 }
 EXPORT_SYMBOL(drm_connector_cleanup);

-- 
2.38.0

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

* Re: [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup
  2022-10-17 15:32 ` [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup Simon Ser
@ 2022-10-17 15:34   ` Jonas Ådahl
  2022-10-17 19:08   ` Lyude Paul
  2022-10-18  9:24   ` Ville Syrjälä
  2 siblings, 0 replies; 5+ messages in thread
From: Jonas Ådahl @ 2022-10-17 15:34 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel, stable, Daniel Vetter, Lyude Paul

On Mon, Oct 17, 2022 at 03:32:01PM +0000, Simon Ser wrote:
> A typical DP-MST unplug removes a KMS connector. However care must
> be taken to properly synchronize with user-space. The expected
> sequence of events is the following:
> 
> 1. The kernel notices that the DP-MST port is gone.
> 2. The kernel marks the connector as disconnected, then sends a
>    uevent to make user-space re-scan the connector list.
> 3. User-space notices the connector goes from connected to disconnected,
>    disables it.
> 4. Kernel handles the the IOCTL disabling the connector. On success,
>    the very last reference to the struct drm_connector is dropped and
>    drm_connector_cleanup() is called.
> 5. The connector is removed from the list, and a uevent is sent to tell
>    user-space that the connector disappeared.
> 
> The very last step was missing. As a result, user-space thought the
> connector still existed and could try to disable it again. Since the
> kernel no longer knows about the connector, that would end up with
> EINVAL and confused user-space.
> 
> Fix this by sending a hotplug uevent from drm_connector_cleanup().
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: stable@vger.kernel.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Jonas Ådahl <jadahl@redhat.com>

Tested-by: Jonas Ådahl <jadahl@redhat.com>


Jonas


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

* Re: [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup
  2022-10-17 15:32 ` [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup Simon Ser
  2022-10-17 15:34   ` Jonas Ådahl
@ 2022-10-17 19:08   ` Lyude Paul
  2022-10-18  9:24   ` Ville Syrjälä
  2 siblings, 0 replies; 5+ messages in thread
From: Lyude Paul @ 2022-10-17 19:08 UTC (permalink / raw)
  To: Simon Ser, dri-devel; +Cc: stable, Daniel Vetter, Jonas Ådahl

LGTM! Thank you for the help with this:

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Mon, 2022-10-17 at 15:32 +0000, Simon Ser wrote:
> A typical DP-MST unplug removes a KMS connector. However care must
> be taken to properly synchronize with user-space. The expected
> sequence of events is the following:
> 
> 1. The kernel notices that the DP-MST port is gone.
> 2. The kernel marks the connector as disconnected, then sends a
>    uevent to make user-space re-scan the connector list.
> 3. User-space notices the connector goes from connected to disconnected,
>    disables it.
> 4. Kernel handles the the IOCTL disabling the connector. On success,
>    the very last reference to the struct drm_connector is dropped and
>    drm_connector_cleanup() is called.
> 5. The connector is removed from the list, and a uevent is sent to tell
>    user-space that the connector disappeared.
> 
> The very last step was missing. As a result, user-space thought the
> connector still existed and could try to disable it again. Since the
> kernel no longer knows about the connector, that would end up with
> EINVAL and confused user-space.
> 
> Fix this by sending a hotplug uevent from drm_connector_cleanup().
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: stable@vger.kernel.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Jonas Ådahl <jadahl@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index e3142c8142b3..90dad87e9ad0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -582,6 +582,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
>  	mutex_destroy(&connector->mutex);
>  
>  	memset(connector, 0, sizeof(*connector));
> +
> +	if (dev->registered)
> +		drm_sysfs_hotplug_event(dev);
>  }
>  EXPORT_SYMBOL(drm_connector_cleanup);
>  

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat


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

* Re: [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup
  2022-10-17 15:32 ` [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup Simon Ser
  2022-10-17 15:34   ` Jonas Ådahl
  2022-10-17 19:08   ` Lyude Paul
@ 2022-10-18  9:24   ` Ville Syrjälä
  2022-10-18  9:26     ` Simon Ser
  2 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2022-10-18  9:24 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel, Daniel Vetter, Jonas Ådahl, stable

On Mon, Oct 17, 2022 at 03:32:01PM +0000, Simon Ser wrote:
> A typical DP-MST unplug removes a KMS connector. However care must
> be taken to properly synchronize with user-space. The expected
> sequence of events is the following:
> 
> 1. The kernel notices that the DP-MST port is gone.
> 2. The kernel marks the connector as disconnected, then sends a
>    uevent to make user-space re-scan the connector list.
> 3. User-space notices the connector goes from connected to disconnected,
>    disables it.
> 4. Kernel handles the the IOCTL disabling the connector. On success,
>    the very last reference to the struct drm_connector is dropped and
>    drm_connector_cleanup() is called.
> 5. The connector is removed from the list, and a uevent is sent to tell
>    user-space that the connector disappeared.
> 
> The very last step was missing. As a result, user-space thought the
> connector still existed and could try to disable it again. Since the
> kernel no longer knows about the connector, that would end up with
> EINVAL and confused user-space.

So is the uevent sent by the mst delayed destroy work
useless now, or what is going on there?

> 
> Fix this by sending a hotplug uevent from drm_connector_cleanup().
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: stable@vger.kernel.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Jonas Ådahl <jadahl@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index e3142c8142b3..90dad87e9ad0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -582,6 +582,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
>  	mutex_destroy(&connector->mutex);
>  
>  	memset(connector, 0, sizeof(*connector));
> +
> +	if (dev->registered)
> +		drm_sysfs_hotplug_event(dev);
>  }
>  EXPORT_SYMBOL(drm_connector_cleanup);
>  
> -- 
> 2.38.0
> 

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup
  2022-10-18  9:24   ` Ville Syrjälä
@ 2022-10-18  9:26     ` Simon Ser
  0 siblings, 0 replies; 5+ messages in thread
From: Simon Ser @ 2022-10-18  9:26 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: dri-devel, Daniel Vetter, Jonas Ådahl, stable

On Tuesday, October 18th, 2022 at 11:24, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Mon, Oct 17, 2022 at 03:32:01PM +0000, Simon Ser wrote:
> 
> > A typical DP-MST unplug removes a KMS connector. However care must
> > be taken to properly synchronize with user-space. The expected
> > sequence of events is the following:
> > 
> > 1. The kernel notices that the DP-MST port is gone.
> > 2. The kernel marks the connector as disconnected, then sends a
> > uevent to make user-space re-scan the connector list.
> > 3. User-space notices the connector goes from connected to disconnected,
> > disables it.
> > 4. Kernel handles the the IOCTL disabling the connector. On success,
> > the very last reference to the struct drm_connector is dropped and
> > drm_connector_cleanup() is called.
> > 5. The connector is removed from the list, and a uevent is sent to tell
> > user-space that the connector disappeared.
> > 
> > The very last step was missing. As a result, user-space thought the
> > connector still existed and could try to disable it again. Since the
> > kernel no longer knows about the connector, that would end up with
> > EINVAL and confused user-space.
> 
> So is the uevent sent by the mst delayed destroy work
> useless now, or what is going on there?

No, that one is still useful, step (2) above.

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

end of thread, other threads:[~2022-10-18  9:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20221017153150.60675-1-contact@emersion.fr>
2022-10-17 15:32 ` [PATCH 2/2] drm/connector: send hotplug uevent on connector cleanup Simon Ser
2022-10-17 15:34   ` Jonas Ådahl
2022-10-17 19:08   ` Lyude Paul
2022-10-18  9:24   ` Ville Syrjälä
2022-10-18  9:26     ` Simon Ser

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