Re: [PATCH] drm: Fix possible race conditions while unplugging DRM device

From: Oleksandr Andrushchenko
Date: Thu May 31 2018 - 03:48:28 EST


On 05/29/2018 10:49 AM, Daniel Vetter wrote:
On Tue, May 29, 2018 at 10:09:57AM +0300, Oleksandr Andrushchenko wrote:
On 05/29/2018 10:02 AM, Daniel Vetter wrote:
On Tue, May 22, 2018 at 05:13:04PM +0300, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

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.

Fixes: bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged")

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
Reported-by: Andrii Chepurnyi <andrii_chepurnyi@xxxxxxxx>
Cc: "Noralf TrÃnnes" <noralf@xxxxxxxxxxx>
Nice catch.

Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>

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
-Daniel

---
drivers/gpu/drm/drm_drv.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index b553a6f2ff0e..7af748ed1c58 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -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);
--
2.17.0

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel