On 9 December 2016 at 14:19, Daniel Vetter daniel.vetter@ffwll.ch wrote:
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 daniel.vetter@ffwll.ch Date: Fri Jun 17 09:33:21 2016 +0200
drm: Protect authmagic with master_mutexCc: Chris Wilson chris@chris-wilson.co.uk Cc: Emil Velikov emil.l.velikov@gmail.com
There's a comment/wild idea below, but the patch itself is
Reviewed-by: Emil Velikov emil.l.velikov@gmail.com
@@ -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 @@ static int drm_setversion(struct drm_device *dev, void *data, struct drm_file *f 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);
Was going to shout "error paths" only to realise that we clobber the user provided storage, even on error. Realistically one could use that info, but from a _very_ quick look I did see any.
Seems like we have all the "used by KMS drivers" ioctls converted to DRM_UNLOCKED, so I'm just wondering: Is it worth, respinning things to: - annotate the legacy-only ioctls (mostly as sanity-check) and do global lock on those, dropping any DRM_UNLOCKED references. - if !legacy-only ioctl, bail out from drm_ioctl() (again, mostly a sanity check) and dropping the boiler plate from the individual ioctls. Not too sure if this one is correct though.
if (drm_core_check_feature(dev, DRIVER_MODESET)) return 0;
Thanks Emil