diff mbox series

[libdrm] xf86drm: Add drmIsMaster()

Message ID 20181120033652.29067-1-christopher.halse.rogers@canonical.com (mailing list archive)
State New, archived
Headers show
Series [libdrm] xf86drm: Add drmIsMaster() | expand

Commit Message

Christopher James Halse Rogers Nov. 20, 2018, 3:36 a.m. UTC
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(+)

Comments

Emil Velikov Dec. 17, 2018, 5:35 p.m. UTC | #1
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
Daniel Vetter Dec. 17, 2018, 5:59 p.m. UTC | #2
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
Christopher James Halse Rogers Dec. 18, 2018, 6:07 a.m. UTC | #3
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?
Christopher James Halse Rogers Jan. 22, 2019, 2:26 a.m. UTC | #4
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()?
<html><head></head><body><div class="gmail_quote">On 18 December 2018 7:07:01 pm NZDT, Christopher James Halse Rogers &lt;chris@cooperteam.net&gt; wrote:<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 18 December 2018 4:35:37 am AEDT, Emil Velikov &lt;emil.l.velikov@gmail.com&gt; wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Hi Christopher,<br><br>On Tue, 20 Nov 2018 at 03:37, Christopher James Halse Rogers<br>&lt;christopher.halse.rogers@canonical.com&gt; wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> We can't use drmSetMaster to query whether or not a drm fd is master<br> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.<br><br></blockquote>Can you please mention the exact use case here? You mentioned it over<br>IRC although it'll be nice to have it here for posterity.<br></blockquote><br>Certainly!<br><br>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.<br><br>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.<br><br>&gt;<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is<br> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect<br> whether or not the fd is master.<br><br></blockquote>I'm wondering if we cannot extent DRM_IOCTL_GET_CLIENT or another<br>IOCTL.<br>What do you think? May I interest you in writing an RFC for the<br>kernel-side?<br></blockquote><br>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?<hr>dri-devel mailing list<br>dri-devel@lists.freedesktop.org<br><a href="https://lists.freedesktop.org/mailman/listinfo/dri-devel">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br></pre></blockquote></div><br clear="all">Ping!<br><br>Is there anything more you'd like to know about my use-case here? Are there any objections to adding drmIsMaster()?</body></html>
Daniel Vetter Jan. 22, 2019, 7:55 a.m. UTC | #5
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
diff mbox series

Patch

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