drm: Fix possible race conditions while unplugging DRM device
Subsystems (2)
PATCH 1/1 drm: Fix possible race conditions while unplugging DRM device
From: Oleksandr Andrushchenko <[email protected]>
When unplugging a hotpluggable DRM device we first unregister it with drm_dev_unregister and then set drm_device.unplugged flag which is used to mark device critical sections with drm_dev_enter()/ drm_dev_exit() preventing access to device resources that are not available after the device is gone.
But drm_dev_unregister may lead to hotplug uevent(s) fired to user-space on card and/or connector removal, thus making it possible for user-space to try accessing a disconnected device.
Fix this by first making sure device is properly marked as disconnected and only then unregister it.
--- drivers/gpu/drm/drm_drv.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/gpu/drm/drm_drv.c +7 -7
@@ -369,13 +369,6 @@ EXPORT_SYMBOL(drm_dev_exit);*/ void drm_dev_unplug (struct drm_device *dev) { -drm_dev_unregister (dev); - -mutex_lock (&drm_global_mutex); -if (dev->open_count ==0 ) -drm_dev_put (dev); -mutex_unlock (&drm_global_mutex); -/* * After synchronizing any critical read section is guaranteed to see * the new value of ->unplugged, and any critical section which might
@@ -384,6 +377,13 @@ void drm_dev_unplug(struct drm_device *dev)*/ dev->unplugged = true;synchronize_srcu (&drm_unplug_srcu); + +drm_dev_unregister (dev); + +mutex_lock (&drm_global_mutex); +if (dev->open_count ==0 ) +drm_dev_put (dev); +mutex_unlock (&drm_global_mutex); }EXPORT_SYMBOL (drm_dev_unplug);
Signature · 6 lines
2.17.0 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
ping
Quoted 45 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
When unplugging a hotpluggable DRM device we first unregister it with
drm_dev_unregisterand then set drm_device.unplugged flag which is used to mark device critical sections with drm_dev_enter()/drm_dev_exit()preventing access to device resources that are not available after the device is gone.
Butdrm_dev_unregistermay lead to hotplug uevent(s) fired to user-space on card and/or connector removal, thus making it possible for user-space to try accessing a disconnected device.Fix this by first making sure device is properly marked as disconnected and only then unregister it.
Fixes bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged")Signed-off-by Oleksandr Andrushchenko <[email protected]>Reported-by Andrii Chepurnyi <[email protected]>Cc "Noralf Trønnes" <[email protected]>--- drivers/gpu/drm/drm_drv.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/gpu/drm/drm_drv.c +0
-7@@ -369,13 +369,6 @@ EXPORT_SYMBOL(drm_dev_exit);*/ void drm_dev_unplug (struct drm_device *dev) { -drm_dev_unregister (dev); - -mutex_lock (&drm_global_mutex); -if (dev->open_count ==0 ) -drm_dev_put (dev); -mutex_unlock (&drm_global_mutex); -/* * After synchronizing any critical read section is guaranteed to see * the new value of ->unplugged, and any critical section which might @@ -384,6 +377,13 @@ void drm_dev_unplug(struct drm_device *dev)*/ dev->unplugged = true;synchronize_srcu (&drm_unplug_srcu);
drivers/gpu/drm/drm_drv.c void drm_dev_unplug(struct drm_device *dev) · line 389
Quoted 10 lines
drivers/gpu/drm/drm_drv.c +7
-0+ +drm_dev_unregister (dev); + +mutex_lock (&drm_global_mutex); +if (dev->open_count ==0 ) +drm_dev_put (dev); +mutex_unlock (&drm_global_mutex); }EXPORT_SYMBOL (drm_dev_unplug);
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 11 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
When unplugging a hotpluggable DRM device we first unregister it with
drm_dev_unregisterand then set drm_device.unplugged flag which is used to mark device critical sections with drm_dev_enter()/drm_dev_exit()preventing access to device resources that are not available after the device is gone.
Butdrm_dev_unregistermay lead to hotplug uevent(s) fired to user-space on card and/or connector removal, thus making it possible for user-space to try accessing a disconnected device.
Quoted 8 lines
Fix this by first making sure device is properly marked as disconnected and only then unregister it.
Fixes bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged")Signed-off-by Oleksandr Andrushchenko <[email protected]>Reported-by Andrii Chepurnyi <[email protected]>Cc "Noralf Trønnes" <[email protected]>
Nice catch.
I think you need to push this to drm-misc-next-fixes to make sure it's on the 4.17 train.
-Daniel
Quoted 43 lines
--- drivers/gpu/drm/drm_drv.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/gpu/drm/drm_drv.c +7
-7@@ -369,13 +369,6 @@ EXPORT_SYMBOL(drm_dev_exit);*/ void drm_dev_unplug (struct drm_device *dev) { -drm_dev_unregister (dev); - -mutex_lock (&drm_global_mutex); -if (dev->open_count ==0 ) -drm_dev_put (dev); -mutex_unlock (&drm_global_mutex); -/* * After synchronizing any critical read section is guaranteed to see * the new value of ->unplugged, and any critical section which might @@ -384,6 +377,13 @@ void drm_dev_unplug(struct drm_device *dev)*/ dev->unplugged = true;synchronize_srcu (&drm_unplug_srcu); + +drm_dev_unregister (dev); + +mutex_lock (&drm_global_mutex); +if (dev->open_count ==0 ) +drm_dev_put (dev); +mutex_unlock (&drm_global_mutex); }EXPORT_SYMBOL (drm_dev_unplug);Signature · 6 lines
2.17.0 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 20 lines · Daniel Vetter
Quoted 11 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
When unplugging a hotpluggable DRM device we first unregister it with
drm_dev_unregisterand then set drm_device.unplugged flag which is used to mark device critical sections with drm_dev_enter()/drm_dev_exit()preventing access to device resources that are not available after the device is gone.
Butdrm_dev_unregistermay lead to hotplug uevent(s) fired to user-space on card and/or connector removal, thus making it possible for user-space to try accessing a disconnected device.Quoted 8 lines
Fix this by first making sure device is properly marked as disconnected and only then unregister it.
Fixes bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged")Signed-off-by Oleksandr Andrushchenko <[email protected]>Reported-by Andrii Chepurnyi <[email protected]>Cc "Noralf Trønnes" <[email protected]>
Quoted 6 lines
Nice catch.
Reviewed-by Daniel Vetter <[email protected]>I think you need to push this to drm-misc-next-fixes to make sure it's on the 4.17 train.
Sure, after I have r-b from Noralf
Quoted 38 lines
-Daniel
Quoted 36 lines
--- drivers/gpu/drm/drm_drv.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/gpu/drm/drm_drv.c +7
-7@@ -369,13 +369,6 @@ EXPORT_SYMBOL(drm_dev_exit);*/ void drm_dev_unplug (struct drm_device *dev) { -drm_dev_unregister (dev); - -mutex_lock (&drm_global_mutex); -if (dev->open_count ==0 ) -drm_dev_put (dev); -mutex_unlock (&drm_global_mutex); -/* * After synchronizing any critical read section is guaranteed to see * the new value of ->unplugged, and any critical section which might @@ -384,6 +377,13 @@ void drm_dev_unplug(struct drm_device *dev)*/ dev->unplugged = true;synchronize_srcu (&drm_unplug_srcu); + +drm_dev_unregister (dev); + +mutex_lock (&drm_global_mutex); +if (dev->open_count ==0 ) +drm_dev_put (dev); +mutex_unlock (&drm_global_mutex); }EXPORT_SYMBOL (drm_dev_unplug);
Quoted 7 lines
Quoted 7 lines
Signature · 6 lines
2.17.0 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 21 lines · Oleksandr Andrushchenko
Quoted 20 lines · Daniel Vetter
Quoted 19 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
When unplugging a hotpluggable DRM device we first unregister it with
drm_dev_unregisterand then set drm_device.unplugged flag which is used to mark device critical sections with drm_dev_enter()/drm_dev_exit()preventing access to device resources that are not available after the device is gone.
Butdrm_dev_unregistermay lead to hotplug uevent(s) fired to user-space on card and/or connector removal, thus making it possible for user-space to try accessing a disconnected device.Fix this by first making sure device is properly marked as disconnected and only then unregister it.
Fixes bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged")Signed-off-by Oleksandr Andrushchenko <[email protected]>Reported-by Andrii Chepurnyi <[email protected]>Cc "Noralf Trønnes" <[email protected]>
Quoted 7 lines
Quoted 6 lines
Nice catch.
Reviewed-by Daniel Vetter <[email protected]>I think you need to push this to drm-misc-next-fixes to make sure it's on the 4.17 train.
Sure, after I have r-b from Noralf
Noralf's occasionally occupied with other things and doesn't have time to look at patches. I think it's ok to just push after a few more days, even if he doesn't respond. Same holds for other people really. -Daniel
Quoted 49 lines
Quoted 40 lines
-Daniel
Quoted 38 lines
--- drivers/gpu/drm/drm_drv.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/gpu/drm/drm_drv.c +7
-7@@ -369,13 +369,6 @@ EXPORT_SYMBOL(drm_dev_exit);*/ void drm_dev_unplug (struct drm_device *dev) { -drm_dev_unregister (dev); - -mutex_lock (&drm_global_mutex); -if (dev->open_count ==0 ) -drm_dev_put (dev); -mutex_unlock (&drm_global_mutex); -/* * After synchronizing any critical read section is guaranteed to see * the new value of ->unplugged, and any critical section which might @@ -384,6 +377,13 @@ void drm_dev_unplug(struct drm_device *dev)*/ dev->unplugged = true;synchronize_srcu (&drm_unplug_srcu); + +drm_dev_unregister (dev); + +mutex_lock (&drm_global_mutex); +if (dev->open_count ==0 ) +drm_dev_put (dev); +mutex_unlock (&drm_global_mutex); }EXPORT_SYMBOL (drm_dev_unplug);Signature · 1 line
2.17.0Quoted 4 lines
Quoted 4 lines
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Oleksandr Andrushchenko
AppliedQuoted 26 lines · Daniel Vetter
Quoted 25 lines · Oleksandr Andrushchenko
Quoted 24 lines · Daniel Vetter
Quoted 11 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
When unplugging a hotpluggable DRM device we first unregister it with
drm_dev_unregisterand then set drm_device.unplugged flag which is used to mark device critical sections with drm_dev_enter()/drm_dev_exit()preventing access to device resources that are not available after the device is gone.
Butdrm_dev_unregistermay lead to hotplug uevent(s) fired to user-space on card and/or connector removal, thus making it possible for user-space to try accessing a disconnected device.Quoted 8 lines
Fix this by first making sure device is properly marked as disconnected and only then unregister it.
Fixes bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged")Signed-off-by Oleksandr Andrushchenko <[email protected]>Reported-by Andrii Chepurnyi <[email protected]>Cc "Noralf Trønnes" <[email protected]>Nice catch.
Reviewed-by Daniel Vetter <[email protected]>
Quoted 8 lines
Quoted 3 lines
Quoted 2 lines
I think you need to push this to drm-misc-next-fixes to make sure it's on the 4.17 train.
Sure, after I have r-b from Noralf
Noralf's occasionally occupied with other things and doesn't have time to look at patches. I think it's ok to just push after a few more days, even if he doesn't respond. Same holds for other people really. -Daniel
Applied to drm-misc-next-fixes
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
-Daniel
Quoted 38 lines
--- drivers/gpu/drm/drm_drv.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/gpu/drm/drm_drv.c +7
-7@@ -369,13 +369,6 @@ EXPORT_SYMBOL(drm_dev_exit);*/ void drm_dev_unplug (struct drm_device *dev) { -drm_dev_unregister (dev); - -mutex_lock (&drm_global_mutex); -if (dev->open_count ==0 ) -drm_dev_put (dev); -mutex_unlock (&drm_global_mutex); -/* * After synchronizing any critical read section is guaranteed to see * the new value of ->unplugged, and any critical section which might @@ -384,6 +377,13 @@ void drm_dev_unplug(struct drm_device *dev)*/ dev->unplugged = true;synchronize_srcu (&drm_unplug_srcu); + +drm_dev_unregister (dev); + +mutex_lock (&drm_global_mutex); +if (dev->open_count ==0 ) +drm_dev_put (dev); +mutex_unlock (&drm_global_mutex); }EXPORT_SYMBOL (drm_dev_unplug);Signature · 1 line
2.17.0
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel