On 26/8/21 5:58 pm, Daniel Vetter wrote:
On Thu, Aug 26, 2021 at 10:01:18AM +0800, Desmond Cheong Zhi Xi wrote:
In a future patch, a read lock on drm_device.master_rwsem is held in the ioctl handler before the check for ioctl permissions. However, this inverts the lock hierarchy of drm_global_mutex --> master_rwsem.
To avoid this, we do some prep work to grab the drm_global_mutex before checking for ioctl permissions.
Signed-off-by: Desmond Cheong Zhi Xi desmondcheongzx@gmail.com
drivers/gpu/drm/drm_ioctl.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index d25713b09b80..158629d88319 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -772,19 +772,19 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, if (drm_dev_is_unplugged(dev)) return -ENODEV;
- /* Enforce sane locking for modern driver ioctls. */
- if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED))
Maybe have a local bool locked_ioctl for this so it's extremely clear it's the same condition in both?
Either way: Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thanks for the suggestion and review. Sounds good, I'll update and send out a new version.
(Sorry for delays, been busy with moving)
mutex_lock(&drm_global_mutex);- retcode = drm_ioctl_permit(flags, file_priv); if (unlikely(retcode))
return retcode;
goto out;
- /* Enforce sane locking for modern driver ioctls. */
- if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) ||
(flags & DRM_UNLOCKED))retcode = func(dev, kdata, file_priv);- else {
mutex_lock(&drm_global_mutex);retcode = func(dev, kdata, file_priv);
- retcode = func(dev, kdata, file_priv);
+out:
- if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED)) mutex_unlock(&drm_global_mutex);
- } return retcode; } EXPORT_SYMBOL(drm_ioctl_kernel);
-- 2.25.1