On 9 December 2016 at 14:19, Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxxx>
> Date: Fri Jun 17 09:33:21 2016 +0200
>
> drm: Protect authmagic with master_mutex
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Emil Velikov <emil.l.velikov@xxxxxxxxx>
There's a comment/wild idea below, but the patch itself is
Reviewed-by: Emil Velikov <emil.l.velikov@xxxxxxxxx>
> @@ -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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
