Re: [PATCH v2] xf86drm: Add drmIsMaster()

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jan 23, 2019 at 03:38:45PM +1100, Christopher James Halse Rogers wrote:
> We can't use drmSetMaster to query whether or not a drm fd is master
> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.
> 
> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
> whether or not the fd is master.
> 
> This is useful for code that might get master by open()ing the drm device
> while no other master exists, but can't call drmSetMaster itself because
> it's not running as root or is in a container, where container-root isn't
> real-root.
> 
> v2: Use the AUTH_MAGIC request rather than MODE_ATTACHMODE, as it's more
>     clearly related to master status.
> 
> Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Can I also motivate you for an igt, to make sure this uapi never breaks
again? I think adding a new subtest to core_auth.c would fit well, which
does:

1. open drm fd, check that his IsMaster function returns true (to avoid
dependencies with unreleased libdrm just make a localdrmIsMaster copy,
we'll collect it eventually).

2. keep the first fd open, open a 2nd fd, check that we're _not_ master
anymore.

3. close both fd.

Also, do you have libdrm commit rights to push this and make a release?

Cheers, Daniel

> ---
>  xf86drm.c | 15 +++++++++++++++
>  xf86drm.h |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..adee5bd9 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2741,6 +2741,21 @@ drm_public int drmDropMaster(int fd)
>          return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
>  }
>  
> +drm_public bool drmIsMaster(int fd)
> +{
> +        /* Detect master by attempting something that requires master.
> +         *
> +         * Authenticating magic tokens requires master and 0 is
> +         * guaranteed to be an invalid magic number. Attempting this on
> +         * a master fd will fail therefore fail with EINVAL because 0 is
> +         * invalid.
> +         *
> +         * A non-master fd will fail with EACCESS, as the kernel checks for
> +         * master before attempting to do anything else.
> +         */
> +        return drmAuthMagic(fd, 0) == EINVAL;
> +}
> +
>  drm_public char *drmGetDeviceNameFromFd(int fd)
>  {
>      char name[128];
> diff --git a/xf86drm.h b/xf86drm.h
> index 7773d71a..9e920db9 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -37,6 +37,7 @@
>  #include <stdarg.h>
>  #include <sys/types.h>
>  #include <stdint.h>
> +#include <stdbool.h>
>  #include <drm.h>
>  
>  #if defined(__cplusplus)
> @@ -733,6 +734,7 @@ extern void drmMsg(const char *format, ...) DRM_PRINTFLIKE(1, 2);
>  
>  extern int drmSetMaster(int fd);
>  extern int drmDropMaster(int fd);
> +extern bool drmIsMaster(int fd);
>  
>  #define DRM_EVENT_CONTEXT_VERSION 4
>  
> -- 
> 2.19.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel