From 5acc614ac47465fee6375a9af4740f618830762d Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 10 Dec 2016 22:52:52 +0100 Subject: drm: Protect master->unique with dev->master_mutex No one looks at the major/minor versions except the unique/busid stuff. If we protect that with the master_mutex (since it also affects the unique of each master, oh well) we can mark these two IOCTL with DRM_UNLOCKED. While doing this I realized that the comment for the magic_map is outdated, I've forgotten to update it in: commit d2b34ee62b409a03c6fe43c07b779983be51d017 Author: Daniel Vetter Date: Fri Jun 17 09:33:21 2016 +0200 drm: Protect authmagic with master_mutex Cc: Chris Wilson Cc: Emil Velikov Reviewed-by: Chris Wilson Reviewed-by: Emil Velikov Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20161210215255.7765-1-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_ioctl.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_ioctl.c') diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index fed22c2b98b6..daacef9bf902 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -115,11 +115,15 @@ static int drm_getunique(struct drm_device *dev, void *data, struct drm_unique *u = data; struct drm_master *master = file_priv->master; + mutex_lock(&master->dev->master_mutex); if (u->unique_len >= master->unique_len) { - if (copy_to_user(u->unique, master->unique, master->unique_len)) + if (copy_to_user(u->unique, master->unique, master->unique_len)) { + mutex_unlock(&master->dev->master_mutex); return -EFAULT; + } } u->unique_len = master->unique_len; + mutex_unlock(&master->dev->master_mutex); return 0; } @@ -340,6 +344,7 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f struct drm_set_version *sv = data; int if_version, retcode = 0; + mutex_lock(&dev->master_mutex); if (sv->drm_di_major != -1) { if (sv->drm_di_major != DRM_IF_MAJOR || sv->drm_di_minor < 0 || sv->drm_di_minor > DRM_IF_MINOR) { @@ -374,6 +379,7 @@ done: sv->drm_di_minor = DRM_IF_MINOR; sv->drm_dd_major = dev->driver->major; sv->drm_dd_minor = dev->driver->minor; + mutex_unlock(&dev->master_mutex); return retcode; } @@ -528,7 +534,7 @@ EXPORT_SYMBOL(drm_ioctl_permit); static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version, DRM_UNLOCKED|DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW), - DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0), + DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid, DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_legacy_getmap_ioctl, DRM_UNLOCKED), @@ -536,7 +542,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0), - DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER), + DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_UNLOCKED | DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF(DRM_IOCTL_BLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), -- cgit From dcf727ab5d1702abc9510a1a91afbd671e410b52 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 10 Dec 2016 22:52:53 +0100 Subject: drm: setclientcap doesn't need the drm BKL It only updates per-file feature flags. And all the ioctl which change behaviour depending upon these flags (they're all kms features) do _not_ hold the BKL. Therefor this is pure cargo-cult and can be removed. Note that there's a risk that the ioctl will behave inconsistently when userspace is racing with itself, but that's ok. The only thing it's not allowed to do is oops the kernel, and from an audit all places are safe. v2: Clarify that the inconsistency is only when userspace races (Chris). Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20161210215255.7765-2-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_ioctl.c') diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index daacef9bf902..18329fc91c80 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -541,7 +541,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0), + DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_UNLOCKED | DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), -- cgit From fdd5b877e9ebc2029e1373b4a3cd057329a9ab7a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 10 Dec 2016 22:52:54 +0100 Subject: drm: Enforce BKL-less ioctls for modern drivers With the last round of changes all ioctls called by modern drivers now have their own locking. Everything else is only allowed for legacy drivers and hence the lack of locking doesn't matter. One exception is nouveau, due to the DRIVER_KMS_LEGACY_CONTEXT flag. But that only works its magic on the context and bufs ioctls. And drm_bufs.c is protected with dev->struct_mutex, and drm_context.c by the same and dev->ctxlist_mutex. That should be all safe, and we can finally mandata drm-bkl-less ioctls for everyone! Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20161210215255.7765-3-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_ioctl.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_ioctl.c') diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 18329fc91c80..59b691146033 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -735,9 +735,8 @@ long drm_ioctl(struct file *filp, if (ksize > in_size) memset(kdata + in_size, 0, ksize - in_size); - /* Enforce sane locking for modern driver ioctls. Core ioctls are - * too messy still. */ - if ((!drm_core_check_feature(dev, DRIVER_LEGACY) && is_driver_ioctl) || + /* Enforce sane locking for modern driver ioctls. */ + if (!drm_core_check_feature(dev, DRIVER_LEGACY) || (ioctl->flags & DRM_UNLOCKED)) retcode = func(dev, kdata, file_priv); else { -- cgit From 25a9939c09eae246173d06e0478f0e131d83b55c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 14 Dec 2016 00:08:02 +0100 Subject: drm/irq: drm_legacy_ prefix for legacy ioctls Spotted while auditing our ioctl table. Also nuke the not-really-kerneldoc comments, we don't document internals and definitely don't want to mislead people with the old dragons. I think with this all the legacy ioctls now have proper drm_legacy_ prefixes. Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20161213230814.19598-2-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_internal.h | 8 ++++---- drivers/gpu/drm/drm_ioctl.c | 4 ++-- drivers/gpu/drm/drm_irq.c | 30 ++++-------------------------- 3 files changed, 10 insertions(+), 32 deletions(-) (limited to 'drivers/gpu/drm/drm_ioctl.c') diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index db80ec860e33..a6213f814345 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -58,10 +58,10 @@ extern unsigned int drm_timestamp_monotonic; /* IOCTLS */ int drm_wait_vblank(struct drm_device *dev, void *data, struct drm_file *filp); -int drm_control(struct drm_device *dev, void *data, - struct drm_file *file_priv); -int drm_modeset_ctl(struct drm_device *dev, void *data, - struct drm_file *file_priv); +int drm_legacy_irq_control(struct drm_device *dev, void *data, + struct drm_file *file_priv); +int drm_legacy_modeset_ctl(struct drm_device *dev, void *data, + struct drm_file *file_priv); /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 59b691146033..d180673c1323 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -581,7 +581,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_FREE_BUFS, drm_legacy_freebufs, DRM_AUTH), DRM_IOCTL_DEF(DRM_IOCTL_DMA, drm_legacy_dma_ioctl, DRM_AUTH), - DRM_IOCTL_DEF(DRM_IOCTL_CONTROL, drm_control, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), + DRM_IOCTL_DEF(DRM_IOCTL_CONTROL, drm_legacy_irq_control, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), #if IS_ENABLED(CONFIG_AGP) DRM_IOCTL_DEF(DRM_IOCTL_AGP_ACQUIRE, drm_agp_acquire_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), @@ -599,7 +599,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0), + DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_legacy_modeset_ctl, 0), DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 273625a85036..feb091310ffe 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -579,19 +579,8 @@ int drm_irq_uninstall(struct drm_device *dev) } EXPORT_SYMBOL(drm_irq_uninstall); -/* - * IRQ control ioctl. - * - * \param inode device inode. - * \param file_priv DRM file private. - * \param cmd command. - * \param arg user argument, pointing to a drm_control structure. - * \return zero on success or a negative number on failure. - * - * Calls irq_install() or irq_uninstall() according to \p arg. - */ -int drm_control(struct drm_device *dev, void *data, - struct drm_file *file_priv) +int drm_legacy_irq_control(struct drm_device *dev, void *data, + struct drm_file *file_priv) { struct drm_control *ctl = data; int ret = 0, irq; @@ -1442,19 +1431,8 @@ static void drm_legacy_vblank_post_modeset(struct drm_device *dev, } } -/* - * drm_modeset_ctl - handle vblank event counter changes across mode switch - * @DRM_IOCTL_ARGS: standard ioctl arguments - * - * Applications should call the %_DRM_PRE_MODESET and %_DRM_POST_MODESET - * ioctls around modesetting so that any lost vblank events are accounted for. - * - * Generally the counter will reset across mode sets. If interrupts are - * enabled around this call, we don't have to do anything since the counter - * will have already been incremented. - */ -int drm_modeset_ctl(struct drm_device *dev, void *data, - struct drm_file *file_priv) +int drm_legacy_modeset_ctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) { struct drm_modeset_ctl *modeset = data; unsigned int pipe; -- cgit From 8caead148a45764bf4adb33220271d0266b85fe5 Mon Sep 17 00:00:00 2001 From: Gabriel Krisman Bertazi Date: Wed, 28 Dec 2016 12:32:13 -0200 Subject: drm: Drop unused forward declaration of drm_version Signed-off-by: Gabriel Krisman Bertazi Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20161228143216.26821-4-krisman@collabora.co.uk --- drivers/gpu/drm/drm_ioctl.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/gpu/drm/drm_ioctl.c') diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index d180673c1323..5c80e720aeca 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -95,9 +95,6 @@ * broken. */ -static int drm_version(struct drm_device *dev, void *data, - struct drm_file *file_priv); - /* * Get the bus id. * -- cgit From 5bbf92d3eb01cb5adcf613e2532dc918172c598f Mon Sep 17 00:00:00 2001 From: Gabriel Krisman Bertazi Date: Wed, 28 Dec 2016 12:32:14 -0200 Subject: drm: Export drm_ioctl_permit to kernel-doc drm_ioctl_permit is exported but missed a kernel-doc style documentation. Signed-off-by: Gabriel Krisman Bertazi Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20161228143216.26821-5-krisman@collabora.co.uk --- drivers/gpu/drm/drm_ioctl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_ioctl.c') diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 5c80e720aeca..a7c61c23685a 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -478,15 +478,17 @@ static int drm_version(struct drm_device *dev, void *data, return err; } -/* +/** * drm_ioctl_permit - Check ioctl permissions against caller * * @flags: ioctl permission flags. * @file_priv: Pointer to struct drm_file identifying the caller. * * Checks whether the caller is allowed to run an ioctl with the - * indicated permissions. If so, returns zero. Otherwise returns an - * error code suitable for ioctl return. + * indicated permissions. + * + * Returns: + * Zero if allowed, -EACCES otherwise. */ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { -- cgit