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);
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:
- The kernel notices that the DP-MST port is gone.
- The kernel marks the connector as disconnected, then sends a uevent to make user-space re-scan the connector list.
- User-space notices the connector goes from connected to disconnected, disables it.
- 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.
- 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
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:
- The kernel notices that the DP-MST port is gone.
- The kernel marks the connector as disconnected, then sends a uevent to make user-space re-scan the connector list.
- User-space notices the connector goes from connected to disconnected, disables it.
- 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.
- 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);
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:
- The kernel notices that the DP-MST port is gone.
- The kernel marks the connector as disconnected, then sends a uevent to make user-space re-scan the connector list.
- User-space notices the connector goes from connected to disconnected, disables it.
- 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.
- 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
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:
- The kernel notices that the DP-MST port is gone.
- 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.
linux-stable-mirror@lists.linaro.org
-
Jonas Ådahl -
Lyude Paul -
Simon Ser -
Ville Syrjälä