[v2] drm: avoid races with modesetting rights

Message ID	20210815153740.195330-1-desmondcheongzx@gmail.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=nuAv=NG=lists.freedesktop.org=dri-devel-bounces@kernel.org> DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C0FF061208 From: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com> To: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch Cc: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, Daniel Vetter <daniel.vetter@ffwll.ch> Subject: [PATCH v2] drm: avoid races with modesetting rights Date: Sun, 15 Aug 2021 23:37:40 +0800 Message-Id: <20210815153740.195330-1-desmondcheongzx@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>
Series	[v2] drm: avoid races with modesetting rights \| expand [v2] drm: avoid races with modesetting rights

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 60a6b21474b1..175bc4d1e4b4 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -29,6 +29,7 @@ */ #include <linux/slab.h> +#include <linux/task_work.h> #include <drm/drm_auth.h> #include <drm/drm_drv.h> @@ -282,6 +283,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, drm_set_master(dev, file_priv, false); out_unlock: mutex_unlock(&dev->master_mutex); + drm_master_flush(dev); return ret; } @@ -321,8 +323,10 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, } drm_drop_master(dev, file_priv); + out_unlock: mutex_unlock(&dev->master_mutex); + drm_master_flush(dev); return ret; } @@ -344,6 +348,8 @@ int drm_master_open(struct drm_file *file_priv) } mutex_unlock(&dev->master_mutex); + drm_master_flush(dev); + return ret; } @@ -450,11 +456,15 @@ EXPORT_SYMBOL(drm_master_put); /* Used by drm_client and drm_fb_helper */ bool drm_master_internal_acquire(struct drm_device *dev) { + down_read(&dev->master_rwsem); + mutex_lock(&dev->master_mutex); if (dev->master) { mutex_unlock(&dev->master_mutex); + up_read(&dev->master_rwsem); return false; } + mutex_unlock(&dev->master_mutex); return true; } @@ -463,6 +473,39 @@ EXPORT_SYMBOL(drm_master_internal_acquire); /* Used by drm_client and drm_fb_helper */ void drm_master_internal_release(struct drm_device *dev) { - mutex_unlock(&dev->master_mutex); + up_read(&dev->master_rwsem); } EXPORT_SYMBOL(drm_master_internal_release); + +/* After flushing, all readers that might have seen old master/lease + * permissions are guaranteed to have completed. + */ +void master_flush(struct callback_head *cb) +{ + struct drm_device *dev = container_of(cb, struct drm_device, + master_flush_work); + + down_write(&dev->master_rwsem); + up_write(&dev->master_rwsem); +} + +/** + * drm_master_flush - queues work to flush readers of master/lease permissions + * before returning to userspace + * @dev: DRM device + * + * Queues up work to flush all readers of master/lease permissions. This work + * is run before this task returns to user mode. Calling this function when a + * task changes modesetting rights ensures that other processes that perform + * modesetting do not race with userspace. + */ +void drm_master_flush(struct drm_device *dev) +{ + init_task_work(&dev->master_flush_work, master_flush); + task_work_add(current, &dev->master_flush_work, TWA_RESUME); + /* If task_work_add fails, then the task is exiting. In this case, it + * doesn't matter if master_flush is run, so we don't need an + * alternative mechanism for flushing. + */ +} +EXPORT_SYMBOL(drm_master_flush); diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7a5097467ba5..10f7e256f999 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -612,6 +612,7 @@ static int drm_dev_init(struct drm_device *dev, mutex_init(&dev->filelist_mutex); mutex_init(&dev->clientlist_mutex); mutex_init(&dev->master_mutex); + init_rwsem(&dev->master_rwsem); ret = drmm_add_action(dev, drm_dev_init_release, NULL); if (ret) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index be4a52dc4d6f..b6e05adecb42 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -785,9 +785,12 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, if (drm_dev_is_unplugged(dev)) return -ENODEV; + if (unlikely(flags & DRM_MASTER)) + down_read(&dev->master_rwsem); + retcode = drm_ioctl_permit(flags, file_priv); if (unlikely(retcode)) - return retcode; + goto release_master; /* Enforce sane locking for modern driver ioctls. */ if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) || @@ -798,6 +801,10 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, retcode = func(dev, kdata, file_priv); mutex_unlock(&drm_global_mutex); } + +release_master: + if (unlikely(flags & DRM_MASTER)) + up_read(&dev->master_rwsem); return retcode; } EXPORT_SYMBOL(drm_ioctl_kernel); diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index dee4f24a1808..983701198ffd 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -723,6 +723,7 @@ int drm_mode_revoke_lease_ioctl(struct drm_device *dev, } _drm_lease_revoke(lessee); + drm_master_flush(dev); fail: mutex_unlock(&dev->mode_config.idr_mutex); diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h index ba248ca8866f..eda3672df6c3 100644 --- a/include/drm/drm_auth.h +++ b/include/drm/drm_auth.h @@ -155,6 +155,7 @@ struct drm_master *drm_master_get(struct drm_master *master); struct drm_master *drm_file_get_master(struct drm_file *file_priv); void drm_master_put(struct drm_master **master); bool drm_is_current_master(struct drm_file *fpriv); +void drm_master_flush(struct drm_device *dev); struct drm_master *drm_master_create(struct drm_device *dev); diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 604b1d1b2d72..eeb58e164788 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -111,6 +111,24 @@ struct drm_device { */ struct drm_master *master; + /** + * @master_rwsem: + * + * Synchronizes modesetting rights between multiple users. Users that + * can change the modeset or display state must hold a read lock on + * @master_rwsem, and users that change modesetting rights should flush + * readers before returning to userspace using drm_master_flush(). + */ + struct rw_semaphore master_rwsem; + + /** + * @master_flush_work: + * + * Callback structure used internally to queue work to flush readers of + * master/lease permissions. + */ + struct callback_head master_flush_work; + /** * @driver_features: per-device driver features *

[v2] drm: avoid races with modesetting rights

Commit Message

Comments

Patch