| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <804f94d8-9f21-159b-220e-ca420a83c176@gmail.com>
Date: Thu, 29 Mar 2018 14:18:44 +0300
From: Oleksandr Andrushchenko <andr2000@...il.com>
To: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
airlied@...ux.ie, daniel.vetter@...el.com, seanpaul@...omium.org,
gustavo@...ovan.org, intel-gfx@...ts.freedesktop.org,
Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
Subject: Re: [PATCH] drm: Use srcu to protect drm_device.unplugged
On 03/28/2018 06:09 PM, Daniel Vetter wrote:
> On Wed, Mar 28, 2018 at 10:38:35AM +0300, Oleksandr Andrushchenko wrote:
>> From: Noralf Trønnes <noralf@...nnes.org>
>>
>> Use srcu to protect drm_device.unplugged in a race free manner.
>> Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
>> sections preventing access to device resources that are not available
>> after the device is gone.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@...ll.ch>
>> Signed-off-by: Noralf Trønnes <noralf@...nnes.org>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
>> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
>> Tested-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
>> Cc: intel-gfx@...ts.freedesktop.org
> Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch>
>
> Oleksandr, please push to drm-misc-next once you have dim tools setup up
> and everything.
Pushed to drm-misc-next
>
> Thanks, Daniel
Thank you,
Oleksandr
>> ---
>> drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
>> include/drm/drm_device.h | 9 +++++++-
>> include/drm/drm_drv.h | 15 +++++++++----
>> 3 files changed, 68 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index a1b9338736e3..32a83b41ab61 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -32,6 +32,7 @@
>> #include <linux/moduleparam.h>
>> #include <linux/mount.h>
>> #include <linux/slab.h>
>> +#include <linux/srcu.h>
>>
>> #include <drm/drm_drv.h>
>> #include <drm/drmP.h>
>> @@ -75,6 +76,8 @@ static bool drm_core_init_complete = false;
>>
>> static struct dentry *drm_debugfs_root;
>>
>> +DEFINE_STATIC_SRCU(drm_unplug_srcu);
>> +
>> /*
>> * DRM Minors
>> * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
>> @@ -318,18 +321,51 @@ void drm_put_dev(struct drm_device *dev)
>> }
>> EXPORT_SYMBOL(drm_put_dev);
>>
>> -static void drm_device_set_unplugged(struct drm_device *dev)
>> +/**
>> + * drm_dev_enter - Enter device critical section
>> + * @dev: DRM device
>> + * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
>> + *
>> + * This function marks and protects the beginning of a section that should not
>> + * be entered after the device has been unplugged. The section end is marked
>> + * with drm_dev_exit(). Calls to this function can be nested.
>> + *
>> + * Returns:
>> + * True if it is OK to enter the section, false otherwise.
>> + */
>> +bool drm_dev_enter(struct drm_device *dev, int *idx)
>> +{
>> + *idx = srcu_read_lock(&drm_unplug_srcu);
>> +
>> + if (dev->unplugged) {
>> + srcu_read_unlock(&drm_unplug_srcu, *idx);
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +EXPORT_SYMBOL(drm_dev_enter);
>> +
>> +/**
>> + * drm_dev_exit - Exit device critical section
>> + * @idx: index returned from drm_dev_enter()
>> + *
>> + * This function marks the end of a section that should not be entered after
>> + * the device has been unplugged.
>> + */
>> +void drm_dev_exit(int idx)
>> {
>> - smp_wmb();
>> - atomic_set(&dev->unplugged, 1);
>> + srcu_read_unlock(&drm_unplug_srcu, idx);
>> }
>> +EXPORT_SYMBOL(drm_dev_exit);
>>
>> /**
>> * drm_dev_unplug - unplug a DRM device
>> * @dev: DRM device
>> *
>> * This unplugs a hotpluggable DRM device, which makes it inaccessible to
>> - * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
>> + * userspace operations. Entry-points can use drm_dev_enter() and
>> + * drm_dev_exit() to protect device resources in a race free manner. This
>> * essentially unregisters the device like drm_dev_unregister(), but can be
>> * called while there are still open users of @dev.
>> */
>> @@ -338,10 +374,18 @@ void drm_dev_unplug(struct drm_device *dev)
>> drm_dev_unregister(dev);
>>
>> mutex_lock(&drm_global_mutex);
>> - drm_device_set_unplugged(dev);
>> 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
>> + * still have seen the old value of ->unplugged is guaranteed to have
>> + * finished.
>> + */
>> + dev->unplugged = true;
>> + synchronize_srcu(&drm_unplug_srcu);
>> }
>> EXPORT_SYMBOL(drm_dev_unplug);
>>
>> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>> index 7c4fa32f3fc6..3a0eac2885b7 100644
>> --- a/include/drm/drm_device.h
>> +++ b/include/drm/drm_device.h
>> @@ -46,7 +46,14 @@ struct drm_device {
>> /* currently active master for this device. Protected by master_mutex */
>> struct drm_master *master;
>>
>> - atomic_t unplugged; /**< Flag whether dev is dead */
>> + /**
>> + * @unplugged:
>> + *
>> + * Flag to tell if the device has been unplugged.
>> + * See drm_dev_enter() and drm_dev_is_unplugged().
>> + */
>> + bool unplugged;
>> +
>> struct inode *anon_inode; /**< inode for private address-space */
>> char *unique; /**< unique name of the device */
>> /*@} */
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index d23dcdd1bd95..7e545f5f94d3 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -624,6 +624,8 @@ void drm_dev_get(struct drm_device *dev);
>> void drm_dev_put(struct drm_device *dev);
>> void drm_dev_unref(struct drm_device *dev);
>> void drm_put_dev(struct drm_device *dev);
>> +bool drm_dev_enter(struct drm_device *dev, int *idx);
>> +void drm_dev_exit(int idx);
>> void drm_dev_unplug(struct drm_device *dev);
>>
>> /**
>> @@ -635,11 +637,16 @@ void drm_dev_unplug(struct drm_device *dev);
>> * unplugged, these two functions guarantee that any store before calling
>> * drm_dev_unplug() is visible to callers of this function after it completes
>> */
>> -static inline int drm_dev_is_unplugged(struct drm_device *dev)
>> +static inline bool drm_dev_is_unplugged(struct drm_device *dev)
>> {
>> - int ret = atomic_read(&dev->unplugged);
>> - smp_rmb();
>> - return ret;
>> + int idx;
>> +
>> + if (drm_dev_enter(dev, &idx)) {
>> + drm_dev_exit(idx);
>> + return false;
>> + }
>> +
>> + return true;
>> }
>>
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@...ts.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Powered by blists - more mailing lists