* [PATCH libdrm] xf86drm: Add drmIsMaster() @ 2018-11-20 3:36 Christopher James Halse Rogers 2018-12-17 17:35 ` Emil Velikov 2019-01-23 4:38 ` [PATCH v2] " Christopher James Halse Rogers 0 siblings, 2 replies; 12+ messages in thread From: Christopher James Halse Rogers @ 2018-11-20 3:36 UTC (permalink / raw) To: dri-devel; +Cc: Christopher James Halse Rogers 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. Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com> --- xf86drm.c | 20 ++++++++++++++++++++ xf86drm.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/xf86drm.c b/xf86drm.c index 10df682b..bdb0439d 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -2741,6 +2741,26 @@ drm_public int drmDropMaster(int fd) return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL); } +drm_public bool drmIsMaster(int fd) +{ + struct drm_mode_mode_cmd cmd; + + memclear(cmd); + /* Set an invalid connector_id to ensure that ATTACHMODE errors with + * EINVAL in the unlikely event someone feels like calling this on a + * kernel prior to 3.9. */ + cmd.connector_id = -1; + + if (drmIoctl(fd, DRM_IOCTL_MODE_ATTACHMODE, &cmd) != -1) + { + /* On 3.9 ATTACHMODE was changed to drm_noop, and so will succeed + * iff we've got a master fd */ + return true; + } + + return errno == 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH libdrm] xf86drm: Add drmIsMaster() 2018-11-20 3:36 [PATCH libdrm] xf86drm: Add drmIsMaster() Christopher James Halse Rogers @ 2018-12-17 17:35 ` Emil Velikov 2018-12-17 17:59 ` Daniel Vetter 2018-12-18 6:07 ` Christopher James Halse Rogers 2019-01-23 4:38 ` [PATCH v2] " Christopher James Halse Rogers 1 sibling, 2 replies; 12+ messages in thread From: Emil Velikov @ 2018-12-17 17:35 UTC (permalink / raw) To: christopher.halse.rogers; +Cc: ML dri-devel Hi Christopher, On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers <christopher.halse.rogers@canonical.com> 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. > Can you please mention the exact use case here? You mentioned it over IRC although it'll be nice to have it here for posterity. > 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. > I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another IOCTL. What do you think? May I interest you in writing an RFC for the kernel-side? IMHO checking with the kernel devs for a cleaner solution is a worthy goal. If they're unhappy we can use this workaround. Thanks Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH libdrm] xf86drm: Add drmIsMaster() 2018-12-17 17:35 ` Emil Velikov @ 2018-12-17 17:59 ` Daniel Vetter 2018-12-18 6:07 ` Christopher James Halse Rogers 1 sibling, 0 replies; 12+ messages in thread From: Daniel Vetter @ 2018-12-17 17:59 UTC (permalink / raw) To: Emil Velikov; +Cc: Christopher James Halse Rogers, ML dri-devel On Mon, Dec 17, 2018 at 6:38 PM Emil Velikov <emil.l.velikov@gmail.com> wrote: > > Hi Christopher, > > On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers > <christopher.halse.rogers@canonical.com> 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. > > > Can you please mention the exact use case here? You mentioned it over > IRC although it'll be nice to have it here for posterity. > > > 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. > > > I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another IOCTL. > What do you think? May I interest you in writing an RFC for the kernel-side? > > IMHO checking with the kernel devs for a cleaner solution is a worthy > goal. If they're unhappy we can use this workaround. I think an invalid request through authmagic would be cleaner. That should give you EINVAL (you're master) or EACCESS (you're not master). It's also directly related to the master status, so less of a mindbinder. Note that we allocate magic numbers through the idr starting at 1, 0 is considered an invalid magic number and rejected. -Daniel > > Thanks > Emil > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH libdrm] xf86drm: Add drmIsMaster() 2018-12-17 17:35 ` Emil Velikov 2018-12-17 17:59 ` Daniel Vetter @ 2018-12-18 6:07 ` Christopher James Halse Rogers 2019-01-22 2:26 ` Christopher James Halse Rogers 1 sibling, 1 reply; 12+ messages in thread From: Christopher James Halse Rogers @ 2018-12-18 6:07 UTC (permalink / raw) To: Emil Velikov, christopher.halse.rogers; +Cc: ML dri-devel On 18 December 2018 4:35:37 am AEDT, Emil Velikov <emil.l.velikov@gmail.com> wrote: >Hi Christopher, > >On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers ><christopher.halse.rogers@canonical.com> 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. >> >Can you please mention the exact use case here? You mentioned it over >IRC although it'll be nice to have it here for posterity. Certainly! The particular use-case I was hitting was testing my display server in a container, where container-root is not real-root but the implicit-master FD you get by opening the DRM node when there is no current master would be sufficient. Just assuming the FD we get is master and failing later breaks the platform probing; for example, when run under X11 if we don't check master we'll load the KMS platform and then fail, rather than noticing that KMS won't work and using our X11 backend. > >> 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. >> >I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another >IOCTL. >What do you think? May I interest you in writing an RFC for the >kernel-side? I think if I was going to do kernel-side changes if probably just make it so that IOCTL_SET_MASTER just unconditionally succeeded on an fd that was already master? _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH libdrm] xf86drm: Add drmIsMaster() 2018-12-18 6:07 ` Christopher James Halse Rogers @ 2019-01-22 2:26 ` Christopher James Halse Rogers 2019-01-22 7:55 ` Daniel Vetter 0 siblings, 1 reply; 12+ messages in thread From: Christopher James Halse Rogers @ 2019-01-22 2:26 UTC (permalink / raw) To: Emil Velikov, christopher.halse.rogers; +Cc: ML dri-devel [-- Attachment #1.1: Type: text/plain, Size: 1965 bytes --] On 18 December 2018 7:07:01 pm NZDT, Christopher James Halse Rogers <chris@cooperteam.net> wrote: >On 18 December 2018 4:35:37 am AEDT, Emil Velikov ><emil.l.velikov@gmail.com> wrote: >>Hi Christopher, >> >>On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers >><christopher.halse.rogers@canonical.com> 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. >>> >>Can you please mention the exact use case here? You mentioned it over >>IRC although it'll be nice to have it here for posterity. > >Certainly! > >The particular use-case I was hitting was testing my display server in >a container, where container-root is not real-root but the >implicit-master FD you get by opening the DRM node when there is no >current master would be sufficient. > >Just assuming the FD we get is master and failing later breaks the >platform probing; for example, when run under X11 if we don't check >master we'll load the KMS platform and then fail, rather than noticing >that KMS won't work and using our X11 backend. > >> >>> 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. >>> >>I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another >>IOCTL. >>What do you think? May I interest you in writing an RFC for the >>kernel-side? > >I think if I was going to do kernel-side changes if probably just make >it so that IOCTL_SET_MASTER just unconditionally succeeded on an fd >that was already master? >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel Ping! Is there anything more you'd like to know about my use-case here? Are there any objections to adding drmIsMaster()? [-- Attachment #1.2: Type: text/html, Size: 2771 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH libdrm] xf86drm: Add drmIsMaster() 2019-01-22 2:26 ` Christopher James Halse Rogers @ 2019-01-22 7:55 ` Daniel Vetter 0 siblings, 0 replies; 12+ messages in thread From: Daniel Vetter @ 2019-01-22 7:55 UTC (permalink / raw) To: Christopher James Halse Rogers Cc: Emil Velikov, Christopher James Halse Rogers, ML dri-devel On Tue, Jan 22, 2019 at 3:26 AM Christopher James Halse Rogers <raof@ubuntu.com> wrote: > > On 18 December 2018 7:07:01 pm NZDT, Christopher James Halse Rogers <chris@cooperteam.net> wrote: >> >> On 18 December 2018 4:35:37 am AEDT, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> >>> Hi Christopher, >>> >>> On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers >>> <christopher.halse.rogers@canonical.com> 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. >>>> >>> Can you please mention the exact use case here? You mentioned it over >>> IRC although it'll be nice to have it here for posterity. >> >> >> Certainly! >> >> The particular use-case I was hitting was testing my display server in a container, where container-root is not real-root but the implicit-master FD you get by opening the DRM node when there is no current master would be sufficient. >> >> Just assuming the FD we get is master and failing later breaks the platform probing; for example, when run under X11 if we don't check master we'll load the KMS platform and then fail, rather than noticing that KMS won't work and using our X11 backend. >> >> > >>>> >>>> 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. >>>> >>> I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another >>> IOCTL. >>> What do you think? May I interest you in writing an RFC for the >>> kernel-side? >> >> >> I think if I was going to do kernel-side changes if probably just make it so that IOCTL_SET_MASTER just unconditionally succeeded on an fd that was already master? Use-case seems all reasonable, I was waiting for a respin with my suggestion ... -Daniel >> ________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > Ping! > > Is there anything more you'd like to know about my use-case here? Are there any objections to adding drmIsMaster()? > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] xf86drm: Add drmIsMaster() 2018-11-20 3:36 [PATCH libdrm] xf86drm: Add drmIsMaster() Christopher James Halse Rogers 2018-12-17 17:35 ` Emil Velikov @ 2019-01-23 4:38 ` Christopher James Halse Rogers 2019-01-23 11:00 ` Daniel Vetter 2019-01-23 17:18 ` Emil Velikov 1 sibling, 2 replies; 12+ messages in thread From: Christopher James Halse Rogers @ 2019-01-23 4:38 UTC (permalink / raw) To: dri-devel; +Cc: Christopher James Halse Rogers 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> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] xf86drm: Add drmIsMaster() 2019-01-23 4:38 ` [PATCH v2] " Christopher James Halse Rogers @ 2019-01-23 11:00 ` Daniel Vetter 2019-01-23 17:18 ` Emil Velikov 1 sibling, 0 replies; 12+ messages in thread From: Daniel Vetter @ 2019-01-23 11:00 UTC (permalink / raw) To: Christopher James Halse Rogers; +Cc: dri-devel 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] xf86drm: Add drmIsMaster() 2019-01-23 4:38 ` [PATCH v2] " Christopher James Halse Rogers 2019-01-23 11:00 ` Daniel Vetter @ 2019-01-23 17:18 ` Emil Velikov 2019-01-23 20:56 ` Christopher James Halse Rogers 1 sibling, 1 reply; 12+ messages in thread From: Emil Velikov @ 2019-01-23 17:18 UTC (permalink / raw) To: Christopher James Halse Rogers; +Cc: ML dri-devel On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers <christopher.halse.rogers@canonical.com> 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> > --- > 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; What magic value is valid, is a DRM implementation detail, which we don't need to depend upon. Instead we can check for EACCES, since we care if we have permissions - aka are we master. The function returns a negative errno, so I'd make this a: return drmAuthMagic(fd, 0) != -EACCES; If you and Daniel agree, I'll squash this locally and push. -Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] xf86drm: Add drmIsMaster() 2019-01-23 17:18 ` Emil Velikov @ 2019-01-23 20:56 ` Christopher James Halse Rogers 2019-01-24 9:45 ` Daniel Vetter 0 siblings, 1 reply; 12+ messages in thread From: Christopher James Halse Rogers @ 2019-01-23 20:56 UTC (permalink / raw) To: dri-devel On 24 January 2019 6:18:32 am NZDT, Emil Velikov <emil.l.velikov@gmail.com> wrote: >On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers ><christopher.halse.rogers@canonical.com> 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> >> --- >> 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; >What magic value is valid, is a DRM implementation detail, which we >don't need to depend upon. > >Instead we can check for EACCES, since we care if we have permissions >- aka are we master. >The function returns a negative errno, so I'd make this a: > > return drmAuthMagic(fd, 0) != -EACCES; > >If you and Daniel agree, I'll squash this locally and push. That's a much better idea, thanks! _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] xf86drm: Add drmIsMaster() 2019-01-23 20:56 ` Christopher James Halse Rogers @ 2019-01-24 9:45 ` Daniel Vetter 2019-02-19 20:40 ` Daniel Vetter 0 siblings, 1 reply; 12+ messages in thread From: Daniel Vetter @ 2019-01-24 9:45 UTC (permalink / raw) To: Christopher James Halse Rogers; +Cc: dri-devel On Thu, Jan 24, 2019 at 09:56:51AM +1300, Christopher James Halse Rogers wrote: > On 24 January 2019 6:18:32 am NZDT, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers > ><christopher.halse.rogers@canonical.com> 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> > >> --- > >> 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; > >What magic value is valid, is a DRM implementation detail, which we > >don't need to depend upon. > > > >Instead we can check for EACCES, since we care if we have permissions > >- aka are we master. > >The function returns a negative errno, so I'd make this a: > > > > return drmAuthMagic(fd, 0) != -EACCES; > > > >If you and Daniel agree, I'll squash this locally and push. > > That's a much better idea, thanks! I don't like checks for "something else happened", I much prefer checking for something specific. Hence == -EINVAL over != EACCESS. But I guess in this case here it doesn't matter. We just need to make sure that the igt tests both ways, to make sure we'll never ever break this. I'd still prefer the current version, imo easier to write an igt to make sure it keeps working. -Daniel -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] xf86drm: Add drmIsMaster() 2019-01-24 9:45 ` Daniel Vetter @ 2019-02-19 20:40 ` Daniel Vetter 0 siblings, 0 replies; 12+ messages in thread From: Daniel Vetter @ 2019-02-19 20:40 UTC (permalink / raw) To: Christopher James Halse Rogers; +Cc: dri-devel On Thu, Jan 24, 2019 at 10:45:04AM +0100, Daniel Vetter wrote: > On Thu, Jan 24, 2019 at 09:56:51AM +1300, Christopher James Halse Rogers wrote: > > On 24 January 2019 6:18:32 am NZDT, Emil Velikov <emil.l.velikov@gmail.com> wrote: > > >On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers > > ><christopher.halse.rogers@canonical.com> 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> > > >> --- > > >> 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; > > >What magic value is valid, is a DRM implementation detail, which we > > >don't need to depend upon. > > > > > >Instead we can check for EACCES, since we care if we have permissions > > >- aka are we master. > > >The function returns a negative errno, so I'd make this a: > > > > > > return drmAuthMagic(fd, 0) != -EACCES; > > > > > >If you and Daniel agree, I'll squash this locally and push. > > > > That's a much better idea, thanks! > > I don't like checks for "something else happened", I much prefer checking > for something specific. Hence == -EINVAL over != EACCESS. But I guess in > this case here it doesn't matter. > > We just need to make sure that the igt tests both ways, to make sure we'll > never ever break this. I'd still prefer the current version, imo easier to > write an igt to make sure it keeps working. Since this landed now ... is the igt anywhere to make sure this keeps working? -Daniel -- 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-02-19 20:40 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-20 3:36 [PATCH libdrm] xf86drm: Add drmIsMaster() Christopher James Halse Rogers 2018-12-17 17:35 ` Emil Velikov 2018-12-17 17:59 ` Daniel Vetter 2018-12-18 6:07 ` Christopher James Halse Rogers 2019-01-22 2:26 ` Christopher James Halse Rogers 2019-01-22 7:55 ` Daniel Vetter 2019-01-23 4:38 ` [PATCH v2] " Christopher James Halse Rogers 2019-01-23 11:00 ` Daniel Vetter 2019-01-23 17:18 ` Emil Velikov 2019-01-23 20:56 ` Christopher James Halse Rogers 2019-01-24 9:45 ` Daniel Vetter 2019-02-19 20:40 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.