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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux