Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 17799 bytes --]

On Thu, 17 Oct 2019 13:45:27 -0400
Drew DeVault <sir@cmpwn.com> wrote:

> From: Marius Vlad <marius.vlad@collabora.com>
> 
> DRM leasing is a feature which allows the DRM master to "lease" a subset
> of its DRM resources to another DRM master via drmModeCreateLease, which
> returns a file descriptor for the new DRM master. We use this protocol
> to negotiate the terms of the lease and transfer this file descriptor to
> clients.
> 
> In less DRM-specific terms: this protocol allows Wayland compositors to
> give over their GPU resources (like displays) to a Wayland client to
> exclusively control.
> 
> The primary use-case for this is Virtual Reality headsets, which via the
> non-desktop DRM property are generally not used as desktop displays by
> Wayland compositors, and for latency reasons (among others) are most
> useful to games et al if they have direct control over the DRM resources
> associated with it. Basically, these are peripherals which are of no use
> to the compositor and may be of use to a client, but since they are tied
> up in DRM we need to use DRM leasing to get them into client's hands.
> 
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> Signed-off-by: Drew DeVault <sir@cmpwn.com>
> ---
> v7's main change is that, upon binding to the drm_lease_manager, the
> server now sends along a non-master DRM fd for the client to use to
> enumerate resources. For this reason, the EDID event has been removed
> from the connector interface, under the assumption that clients who want
> to examine the EDID will line the connnector ID up with the appropriate
> resources from DRM.

Hi Drew,

thanks for this, I really hope it works out since the protocol is so
neat and tidy now.

One thing I did not know the last time was that apparently
systemd-logind may not like to give out non-master DRM fds. That might
need fixing in logind implementations. I hope someone would step up to
look into that.

I'm CC'ing dri-devel and Daniel Vetter to get some kernel side review
for the non-master DRM fd idea:

This protocol aims to deliver a harmless "read-only" DRM device file
description to a client, so that the client can enumerate all DRM
resources, fetch EDID and other properties to be able to decide which
connector it would want to lease. The client should not be able to
change any KMS state through this fd, and it should not be able to e.g.
spy on display contents. The assumption is that a non-master DRM fd
from a fresh open() would be fine for this, but is it?

If it is not, could we make one that is? Simply giving out an fd for
the client to inspect with standard DRM ioctls is just so convenient.


**

I wouldn't mind if the below links were part of the proper commit
message.

> Updated patches are available for:
> 
> wlroots:  https://github.com/swaywm/wlroots/pull/1730
> sway:     https://github.com/swaywm/sway/pull/4289
> kmscube:  https://git.sr.ht/~sircmpwn/kmscube
> Xwayland: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/248
> 
> Additionally, the Vulkan extension has been polished up:
> 
> https://github.com/KhronosGroup/Vulkan-Docs/pull/1001
> 
> A new implementation for Mesa's Vulkan WSI implementation will be
> available soon, as well as an implementation of the Wayland extension
> for Monado.
> 
>  Makefile.am                                  |   1 +
>  unstable/drm-lease/README                    |   5 +
>  unstable/drm-lease/drm-lease-unstable-v1.xml | 246 +++++++++++++++++++
>  3 files changed, 252 insertions(+)
>  create mode 100644 unstable/drm-lease/README
>  create mode 100644 unstable/drm-lease/drm-lease-unstable-v1.xml
> 
> diff --git a/Makefile.am b/Makefile.am
> index 345ae6a..d9fff89 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -4,6 +4,7 @@ unstable_protocols =								\
>  	unstable/pointer-gestures/pointer-gestures-unstable-v1.xml		\
>  	unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml		\
>  	unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml			\
> +	unstable/drm-lease/drm-lease-unstable-v1.xml				\
>  	unstable/text-input/text-input-unstable-v1.xml				\
>  	unstable/text-input/text-input-unstable-v3.xml				\
>  	unstable/input-method/input-method-unstable-v1.xml			\
> diff --git a/unstable/drm-lease/README b/unstable/drm-lease/README
> new file mode 100644
> index 0000000..16f8551
> --- /dev/null
> +++ b/unstable/drm-lease/README
> @@ -0,0 +1,5 @@
> +Linux DRM lease
> +
> +Maintainers:
> +Drew DeVault <sir@cmpwn.com>
> +Marius Vlad <marius-cristian.vlad@nxp.com>

Marius' email address probably needs refreshing?


> diff --git a/unstable/drm-lease/drm-lease-unstable-v1.xml b/unstable/drm-lease/drm-lease-unstable-v1.xml
> new file mode 100644
> index 0000000..5fbc0e3
> --- /dev/null
> +++ b/unstable/drm-lease/drm-lease-unstable-v1.xml
> @@ -0,0 +1,246 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="drm_lease_unstable_v1">
> +  <copyright>
> +    Copyright © 2018 NXP
> +    Copyright © 2019 Status Research &amp; Development GmbH.
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a
> +    copy of this software and associated documentation files (the "Software"),
> +    to deal in the Software without restriction, including without limitation
> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +    and/or sell copies of the Software, and to permit persons to whom the
> +    Software is furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice (including the next
> +    paragraph) shall be included in all copies or substantial portions of the
> +    Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +    DEALINGS IN THE SOFTWARE.
> +  </copyright>
> +
> +  <interface name="zwp_drm_lease_manager_v1" version="1">
> +    <description summary="lease manager">
> +      This protocol is used by Wayland compositors which act as Direct
> +      Renderering Manager (DRM) masters to lease DRM resources to Wayland
> +      clients. Once leased, the compositor will not use the leased resources
> +      until the lease is revoked or the client closes the file descriptor. The
> +      compositor will advertise one zwp_drm_lease_manager_v1 for each DRM node
> +      which has resources available for leasing.

Ok, that takes care of the multi-card case. It also nicely identifies
which device a connector is from.

> +
> +      The lease manager is used to advertise connectors which are available for
> +      leasing, and by the client to negotiate a lease request.
> +
> +      Warning! The protocol described in this file is experimental and
> +      backward incompatible changes may be made. Backward compatible changes
> +      may be added together with the corresponding interface version bump.
> +      Backward incompatible changes are done by bumping the version number in
> +      the protocol and interface names and resetting the interface version.
> +      Once the protocol is to be declared stable, the 'z' prefix and the
> +      version number in the protocol and interface names are removed and the
> +      interface version number is reset.
> +    </description>
> +
> +    <enum name="error">
> +      <entry name="stopped_manager" value="0"
> +        summary="request sent to a manager which has been stopped"/>
> +    </enum>
> +
> +    <request name="create_lease_request">
> +      <description summary="create a lease request object">
> +        Creates a lease request object.
> +
> +        See the documentation for zwp_drm_lease_request_v1 for details.
> +      </description>
> +      <arg name="id" type="new_id" interface="zwp_drm_lease_request_v1" />
> +    </request>
> +
> +    <request name="stop">
> +      <description summary="stop sending events">
> +        Indicates the client no longer wishes to receive connector events. The
> +        compositor may still send connector events until it sends the finish
> +        event, however.
> +
> +        The client must not send any requests after this one.
> +      </description>
> +    </request>
> +
> +    <event name="drm_fd">
> +      <description summary="open a non-master fd for this DRM node">
> +        The compositor will send this event when the zwp_drm_lease_manager_v1
> +        global is bound. The included fd is a non-master DRM file descriptor
> +        opened for this device. The purpose of this event is to give the client
> +        the ability to query DRM and discover information which may help them
> +        pick the appropriate DRM device or select the appropriate connectors
> +        therein.
> +      </description>
> +      <arg name="fd" type="fd" summary="DRM file descriptor" />
> +    </event>
> +
> +    <event name="connector">
> +      <description summary="advertise connectors available for leases">
> +        The compositor may choose to advertise 0 or more connectors which may be
> +        leased to clients, and will use this event to do so. This object may be
> +        passed into a lease request to lease that connector. See
> +        zwp_drm_lease_request_v1.add_connector for details.
> +
> +        When this global is bound, the compositor will send all connectors
> +        available for lease, but may send additional connectors at any time.
> +      </description>
> +      <arg name="id" type="new_id" interface="zwp_drm_lease_connector_v1" />
> +    </event>
> +
> +    <event name="finished">
> +      <description summary="the compositor has finished using the manager">
> +        This event indicates that the compositor is done sending connector
> +        events. The compositor will destroy this object immediately after
> +        sending this event, and it will become invalid. The client should
> +        release any resources associated with this manager after receiving this
> +        event.
> +      </description>
> +    </event>
> +  </interface>
> +
> +  <interface name="zwp_drm_lease_connector_v1" version="1">
> +    <description summary="a leasable DRM connector">
> +      Represents a DRM connector which is available for lease. These objects are
> +      created via zwp_drm_lease_manager_v1.connector, and should be passed into
> +      lease requests via zwp_drm_lease_request_v1.add_connector.
> +    </description>
> +
> +    <event name="name">
> +      <description summary="name">
> +        The compositor sends this event once the connector is created to
> +        indicate the name of this connector. This will not change for the
> +        duration of the Wayland session, but is not guaranteed to be consistent
> +        between sessions.
> +
> +        If the compositor also supports zxdg_output_manager_v1 and this
> +        connector corresponds to a zxdg_output_v1, this name will match the
> +        name of this zxdg_output_v1 object.
> +      </description>
> +      <arg name="name" type="string" summary="connector name" />
> +    </event>
> +
> +    <event name="description">
> +      <description summary="description">
> +        The compositor sends this event once the connector is created to provide
> +        a human-readable description for this connector, which may be presented
> +        to the user.
> +      </description>
> +      <arg name="description" type="string" summary="connector description" />
> +    </event>
> +
> +    <event name="connector_id">
> +      <description summary="connector_id">
> +        The compositor will send this event to indicate the DRM ID which
> +        represents the underlying connector which is being offered. Note that
> +        the final lease may include additional object IDs, such as CRTCs and
> +        planes.
> +      </description>
> +      <arg name="connector_id" type="int" summary="DRM Connector ID" />
> +    </event>
> +
> +    <event name="withdrawn">
> +      <description summary="lease offer withdrawn">
> +        Sent to indicate that the compositor will no longer honor requests for
> +        DRM leases which include this connector. The client may still issue a
> +        lease request including this connector, but the compositor will send
> +        zwp_drm_lease_v1.finished without issuing a lease fd.
> +      </description>
> +    </event>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy connector">
> +        The client may send this request to indicate that it will not issue a
> +        lease request for this connector. Clients are encouraged to send this
> +        after receiving the "withdrawn" request so that the server can release
> +        the resources associated with this connector offer.
> +      </description>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_drm_lease_request_v1" version="1">
> +    <description summary="DRM lease request">
> +      A client that wishes to lease DRM resources will attach the list of
> +      connectors advertised with zwp_drm_lease_manager_v1.connector that they
> +      wish to lease, then use zwp_drm_lease_request_v1.submit to submit the
> +      request.
> +    </description>
> +
> +    <enum name="error">
> +      <entry name="submitted_lease" value="0"
> +        summary="attempted to reuse a submitted lease"/>
> +    </enum>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroys the lease request object">
> +        Indicates that the client will no longer use this lease request.
> +      </description>
> +    </request>
> +
> +    <request name="request_connector">
> +       <description summary="request a connector for this lease">
> +         Indicates that the client would like to lease the given connector.
> +         This is only used as a suggestion, the compositor may choose to
> +         include any resources in the lease it issues, or change the set of
> +         leased resources at any time.
> +       </description>
> +       <arg name="connector" type="object"
> +         interface="zwp_drm_lease_connector_v1" />

Don't we need a protocol error defined for the case, where the client
uses a zwp_drm_lease_connector_v1 from a different global? That is,
attempting to lease a connector from the wrong device.

If that was an error, then it would make sure that one cannot lease a
mix across multiple devices in one go. I don't think leasing a mix
across multiple devices could work, given only one DRM lease fd should
be delivered.

> +    </request>
> +
> +    <request name="submit">
> +       <description summary="submit the lease request">
> +         Submits the lease request and creates a new zwp_drm_lease_v1 object.
> +         After calling submit, issuing any other request than destroy is a
> +         protocol error.
> +       </description>
> +       <arg name="id" type="new_id" interface="zwp_drm_lease_v1" />
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_drm_lease_v1" version="1">
> +    <description summary="a DRM lease">
> +      A DRM lease object is used to transfer the DRM file descriptor to the
> +      client and manage the lifetime of the lease.
> +    </description>
> +
> +    <event name="lease_fd">
> +      <description summary="shares the DRM file descriptor">
> +        This event returns a file descriptor suitable for use with DRM-related
> +        ioctls. The client should use drmModeGetLease to enumerate the DRM
> +        objects which have been leased to them. If the compositor cannot or

Isn't the wording about drmModeGetLease() wrong here? The client can
only see the leased resources through that fd, no matter what it does,
right?

> +        will not grant a lease for the requested connectors, it will not send
> +        this event, instead sending the finished event immediately.
> +
> +        It is a protocol error for the compositor to send this event more than
> +        once for a given lease.
> +      </description>
> +      <arg name="leased_fd" type="fd" summary="leased DRM file descriptor" />
> +    </event>
> +
> +    <event name="finished">
> +      <description summary="sent when the lease has been revoked">
> +        When the compositor revokes the lease, it will issue this event to
> +        notify clients of the change. If the client requires a new lease, they
> +        should destroy this object and submit a new lease request. The
> +        compositor will send no further events for this object after sending
> +        the finish event.
> +      </description>
> +    </event>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroys the lease object">
> +        The client should send this to indicate that it no longer wishes to use
> +        this lease. The compositor should use drmModeRevokeLease on the
> +        appropriate file descriptor, if necessary, then release this object.
> +      </description>
> +    </request>
> +  </interface>
> +</protocol>

This seems to be in a very good shape now.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Drew DeVault]

On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:
> One thing I did not know the last time was that apparently
> systemd-logind may not like to give out non-master DRM fds. That might
> need fixing in logind implementations. I hope someone would step up to
> look into that.
>
> This protocol aims to deliver a harmless "read-only" DRM device file
> description to a client, so that the client can enumerate all DRM
> resources, fetch EDID and other properties to be able to decide which
> connector it would want to lease. The client should not be able to
> change any KMS state through this fd, and it should not be able to e.g.
> spy on display contents. The assumption is that a non-master DRM fd
> from a fresh open() would be fine for this, but is it?

What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
path to the device file, and then I open() it and use
drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
to work correctly in practice.
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 1765 bytes --]

On Fri, 18 Oct 2019 07:54:50 -0400
"Drew DeVault" <sir@cmpwn.com> wrote:

> On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:
> > One thing I did not know the last time was that apparently
> > systemd-logind may not like to give out non-master DRM fds. That might
> > need fixing in logind implementations. I hope someone would step up to
> > look into that.
> >
> > This protocol aims to deliver a harmless "read-only" DRM device file
> > description to a client, so that the client can enumerate all DRM
> > resources, fetch EDID and other properties to be able to decide which
> > connector it would want to lease. The client should not be able to
> > change any KMS state through this fd, and it should not be able to e.g.
> > spy on display contents. The assumption is that a non-master DRM fd
> > from a fresh open() would be fine for this, but is it?  
> 
> What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
> path to the device file, and then I open() it and use
> drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
> to work correctly in practice.

That is nice.

Personally I'm specifically worried about a setup where the user has no
access permissions to open the DRM device node directly, as is (or
should be) the case with input devices.

However, since DRM has the master concept which input devices do not,
maybe there is no reason to prevent a normal user from opening a DRM
device directly. That is, if our assumption that a non-master DRM fd is
harmless holds.

(Wayland display servers usually run as a normal user, while logind
or another service runs with elevated privileges and opens input and
DRM devices on behalf of the display server.)


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Ville Syrjälä]

On Fri, Oct 18, 2019 at 04:43:29PM +0300, Pekka Paalanen wrote:
> On Fri, 18 Oct 2019 07:54:50 -0400
> "Drew DeVault" <sir@cmpwn.com> wrote:
> 
> > On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:
> > > One thing I did not know the last time was that apparently
> > > systemd-logind may not like to give out non-master DRM fds. That might
> > > need fixing in logind implementations. I hope someone would step up to
> > > look into that.
> > >
> > > This protocol aims to deliver a harmless "read-only" DRM device file
> > > description to a client, so that the client can enumerate all DRM
> > > resources, fetch EDID and other properties to be able to decide which
> > > connector it would want to lease. The client should not be able to
> > > change any KMS state through this fd, and it should not be able to e.g.
> > > spy on display contents. The assumption is that a non-master DRM fd
> > > from a fresh open() would be fine for this, but is it?  
> > 
> > What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
> > path to the device file, and then I open() it and use
> > drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
> > to work correctly in practice.
> 
> That is nice.
> 
> Personally I'm specifically worried about a setup where the user has no
> access permissions to open the DRM device node directly, as is (or
> should be) the case with input devices.
> 
> However, since DRM has the master concept which input devices do not,
> maybe there is no reason to prevent a normal user from opening a DRM
> device directly. That is, if our assumption that a non-master DRM fd is
> harmless holds.

If there is no master then the first guy to open the device
automagically becomes the master. Maybe a theoretical DoS
vector if you can sneak in and open the device between
dropmaster/setmaster?

-- 
Ville Syrjälä
Intel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Daniel Vetter]

On Fri, Oct 18, 2019 at 3:43 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Fri, 18 Oct 2019 07:54:50 -0400
> "Drew DeVault" <sir@cmpwn.com> wrote:
>
> > On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:
> > > One thing I did not know the last time was that apparently
> > > systemd-logind may not like to give out non-master DRM fds. That might
> > > need fixing in logind implementations. I hope someone would step up to
> > > look into that.
> > >
> > > This protocol aims to deliver a harmless "read-only" DRM device file
> > > description to a client, so that the client can enumerate all DRM
> > > resources, fetch EDID and other properties to be able to decide which
> > > connector it would want to lease. The client should not be able to
> > > change any KMS state through this fd, and it should not be able to e.g.
> > > spy on display contents. The assumption is that a non-master DRM fd
> > > from a fresh open() would be fine for this, but is it?
> >
> > What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
> > path to the device file, and then I open() it and use
> > drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
> > to work correctly in practice.
>
> That is nice.
>
> Personally I'm specifically worried about a setup where the user has no
> access permissions to open the DRM device node directly, as is (or
> should be) the case with input devices.
>
> However, since DRM has the master concept which input devices do not,
> maybe there is no reason to prevent a normal user from opening a DRM
> device directly. That is, if our assumption that a non-master DRM fd is
> harmless holds.
>
> (Wayland display servers usually run as a normal user, while logind
> or another service runs with elevated privileges and opens input and
> DRM devices on behalf of the display server.)

So the rules are (if I'm not making a mistake)
- If you're not CAP_SYS_ADMIN you can't get/drop_master.

- This is a bit awkward, since non-root can become a master, when
there's no other master right at this point. So if you want to be able
to do this, we should probably clarify this part of the uapi somehow
(either de-priv drop_master or make sure non-root can't become master,
but the latter probably will break something somewhere). Plus igts to
lock down this behaviour. Note that if logind does a vt switch there's
a race window where no one is master and you might be able to squeeze
in. So perhaps we do want to stop this behaviour and require
CAP_SYS_ADMIN to become master, even accidentally.

- I thought you can always re-open your own fd through proc? Which
should be good enough for this use-case here.

- Non-master primary node should indeed give you all the GET* ioctls
for kms, and nothing else useful or at least dangerous (you might be
able to render with that thing). Just make sure you dont authenticate
that new fd. Again maybe we should clarify our docs a bit to make this
use case official.

Cheers, Daniel
-- 
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

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 4331 bytes --]

On Fri, 18 Oct 2019 16:19:33 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> On Fri, Oct 18, 2019 at 3:43 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > On Fri, 18 Oct 2019 07:54:50 -0400
> > "Drew DeVault" <sir@cmpwn.com> wrote:
> >  
> > > On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:  
> > > > One thing I did not know the last time was that apparently
> > > > systemd-logind may not like to give out non-master DRM fds. That might
> > > > need fixing in logind implementations. I hope someone would step up to
> > > > look into that.
> > > >
> > > > This protocol aims to deliver a harmless "read-only" DRM device file
> > > > description to a client, so that the client can enumerate all DRM
> > > > resources, fetch EDID and other properties to be able to decide which
> > > > connector it would want to lease. The client should not be able to
> > > > change any KMS state through this fd, and it should not be able to e.g.
> > > > spy on display contents. The assumption is that a non-master DRM fd
> > > > from a fresh open() would be fine for this, but is it?  
> > >
> > > What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
> > > path to the device file, and then I open() it and use
> > > drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
> > > to work correctly in practice.  
> >
> > That is nice.
> >
> > Personally I'm specifically worried about a setup where the user has no
> > access permissions to open the DRM device node directly, as is (or
> > should be) the case with input devices.
> >
> > However, since DRM has the master concept which input devices do not,
> > maybe there is no reason to prevent a normal user from opening a DRM
> > device directly. That is, if our assumption that a non-master DRM fd is
> > harmless holds.
> >
> > (Wayland display servers usually run as a normal user, while logind
> > or another service runs with elevated privileges and opens input and
> > DRM devices on behalf of the display server.)  
> 
> So the rules are (if I'm not making a mistake)
> - If you're not CAP_SYS_ADMIN you can't get/drop_master.

Hi,

not able to drop, yikes. So if someone pokes the Wayland DRM leasing
interface while the display server is VT switched away (does not have
DRM master), and maybe no-one else has DRM master either (you're
hacking in VT text mode), then a new DRM fd would be master with no way
out?

So Wayland display servers should make sure they have master themselves
before sending a supposedly non-master DRM fd to anyone else. I wonder
if the Wayland protocol extension needs to consider that the compositor
might not be able to send any fd soon. Being able to defer sending the
fd should probably be mentioned in the protocol spec, so that clients
do not expect a simple roundtrip to be enough to ensure the fd has
arrived.

> - This is a bit awkward, since non-root can become a master, when
> there's no other master right at this point. So if you want to be able
> to do this, we should probably clarify this part of the uapi somehow
> (either de-priv drop_master or make sure non-root can't become master,
> but the latter probably will break something somewhere). Plus igts to
> lock down this behaviour. Note that if logind does a vt switch there's
> a race window where no one is master and you might be able to squeeze
> in. So perhaps we do want to stop this behaviour and require
> CAP_SYS_ADMIN to become master, even accidentally.

That would close the loophole that Ville mentioned, too, otherwise
distributions should aim to not give permissions to open the DRM device
node.

> - I thought you can always re-open your own fd through proc? Which
> should be good enough for this use-case here.

We can? And that creates a new file description the same way as open()
in the original device node?

Does that avoid becoming master in the above VT-switched-away scenario?

> - Non-master primary node should indeed give you all the GET* ioctls
> for kms, and nothing else useful or at least dangerous (you might be
> able to render with that thing). Just make sure you dont authenticate
> that new fd. Again maybe we should clarify our docs a bit to make this
> use case official.

Awesome, thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Drew DeVault]

Regarding hotplugging, the Wayland compositor is probably keeping track
of hotplugs itself and withdrawing/offering connectors as appropriate.
Also, when the lease is issued, the compositor withdraws that connector.
For the client, upon hotplug I imagine the DRM asks start to fail, and
it handles that accordingly (presumably it'll close the lease, if the
compositor hasn't already, and wait for it to come back, or just exit).

When a hotplug of a leased connector happens on the compositor side, it
can notice this and exercise its descretion in the implementation. I
think the current text of the protocol supports this view.

On Fri Oct 18, 2019 at 5:34 PM Pekka Paalanen wrote:
> > So the rules are (if I'm not making a mistake)
> > - If you're not CAP_SYS_ADMIN you can't get/drop_master.
> 
> not able to drop, yikes. So if someone pokes the Wayland DRM leasing
> interface while the display server is VT switched away (does not have
> DRM master), and maybe no-one else has DRM master either (you're
> hacking in VT text mode), then a new DRM fd would be master with no way
> out?

fwiw I have an assert(!drmIsMaster(fd)); before I send it to the client,
just to be safe. But this may be misguided, since apparently if it ends
up with a master node drmDropMaster(fd) won't work. I kind of find this
hard to believe, if there's only ever one master, and the master cannot
drop master, then why does this ioctl even exist?

> So Wayland display servers should make sure they have master themselves
> before sending a supposedly non-master DRM fd to anyone else. I wonder
> if the Wayland protocol extension needs to consider that the compositor
> might not be able to send any fd soon. Being able to defer sending the
> fd should probably be mentioned in the protocol spec, so that clients
> do not expect a simple roundtrip to be enough to ensure the fd has
> arrived.

When you VT switch away, the Wayland compositor is no longer necessarily
able to lease those displays anyway - it's not the master anymore. So it
should withdraw them, and in case of a race it'll reject the lease
request.
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Philipp Zabel]

On Fri, 2019-10-18 at 10:43 -0400, Drew DeVault wrote:
> Regarding hotplugging, the Wayland compositor is probably keeping track
> of hotplugs itself and withdrawing/offering connectors as appropriate.
>
> Also, when the lease is issued, the compositor withdraws that connector.
> For the client, upon hotplug I imagine the DRM asks start to fail, and
> it handles that accordingly (presumably it'll close the lease, if the
> compositor hasn't already, and wait for it to come back, or just exit).

Right. Whether it waits or quits should be the decision of the client,
and I'd like there to be a good way to "wait for it to come back" (or to
appear, initially).
If the compositor sends a new zwp_drm_lease_manager_v1.connector event
after the HMD connector becomes leasable (again), that should be good
enough.

[...]
> > So Wayland display servers should make sure they have master themselves
> > before sending a supposedly non-master DRM fd to anyone else. I wonder
> > if the Wayland protocol extension needs to consider that the compositor
> > might not be able to send any fd soon. Being able to defer sending the
> > fd should probably be mentioned in the protocol spec, so that clients
> > do not expect a simple roundtrip to be enough to ensure the fd has
> > arrived.
> 
> When you VT switch away, the Wayland compositor is no longer necessarily
> able to lease those displays anyway - it's not the master anymore. So it
> should withdraw them, and in case of a race it'll reject the lease
> request.

Right. On VT switch away, revoking all leases and disabling those
connectors is the only sensible thing the compositor can do.

However, that is completely independent from the sending the drm_fd
event. The spec currently says: "The compositor will send this event
when the zwp_drm_lease_manager_v1 global is bound.". But if the client
binds the global while the compositor doesn't have DRM master privilege,
and if it is not possible to securely produce a non-master drm_fd to
send at this time, maybe the sentence should be changed to "The
compositor will send this event some time after the
zwp_drm_lease_manager_v1 global is bound."?

regards
Philipp

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 1704 bytes --]

On Fri, 18 Oct 2019 10:43:16 -0400
"Drew DeVault" <sir@cmpwn.com> wrote:

> Regarding hotplugging, the Wayland compositor is probably keeping track
> of hotplugs itself and withdrawing/offering connectors as appropriate.
> Also, when the lease is issued, the compositor withdraws that connector.
> For the client, upon hotplug I imagine the DRM asks start to fail, and
> it handles that accordingly (presumably it'll close the lease, if the
> compositor hasn't already, and wait for it to come back, or just exit).

DRM KMS operations do not fail merely because a connector becomes
disconnected. You can even force on a connector that is initially
disconnected.

If you mean on revoking the lease or losing DRM master, yes, then I'd
expect KMS ioctls start to fail.

But is disconnection reason enough to revoke the lease? I guess we
shall see.

> When a hotplug of a leased connector happens on the compositor side, it
> can notice this and exercise its descretion in the implementation. I
> think the current text of the protocol supports this view.

Ok, so the expectation is that a compositor does not offer disconnected
connectors, and withdraws offered but then disconnected connectors, and
that it sends offers for connectors that become connected while
leasable. I suppose that is reasonable, I still think a sentence
suggesting towards such behaviour would be in place.

Btw. there is more to hotplug events than just connected/disconnected:
link status changes, and HDCP status changes. I suspect more is coming,
too. At some point we might need to add a hotplug event in the
protocol, but I think that is easy to do even after stabilization.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Daniel Vetter]

On Fri, Oct 18, 2019 at 4:34 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Fri, 18 Oct 2019 16:19:33 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > On Fri, Oct 18, 2019 at 3:43 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > >
> > > On Fri, 18 Oct 2019 07:54:50 -0400
> > > "Drew DeVault" <sir@cmpwn.com> wrote:
> > >
> > > > On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:
> > > > > One thing I did not know the last time was that apparently
> > > > > systemd-logind may not like to give out non-master DRM fds. That might
> > > > > need fixing in logind implementations. I hope someone would step up to
> > > > > look into that.
> > > > >
> > > > > This protocol aims to deliver a harmless "read-only" DRM device file
> > > > > description to a client, so that the client can enumerate all DRM
> > > > > resources, fetch EDID and other properties to be able to decide which
> > > > > connector it would want to lease. The client should not be able to
> > > > > change any KMS state through this fd, and it should not be able to e.g.
> > > > > spy on display contents. The assumption is that a non-master DRM fd
> > > > > from a fresh open() would be fine for this, but is it?
> > > >
> > > > What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
> > > > path to the device file, and then I open() it and use
> > > > drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
> > > > to work correctly in practice.
> > >
> > > That is nice.
> > >
> > > Personally I'm specifically worried about a setup where the user has no
> > > access permissions to open the DRM device node directly, as is (or
> > > should be) the case with input devices.
> > >
> > > However, since DRM has the master concept which input devices do not,
> > > maybe there is no reason to prevent a normal user from opening a DRM
> > > device directly. That is, if our assumption that a non-master DRM fd is
> > > harmless holds.
> > >
> > > (Wayland display servers usually run as a normal user, while logind
> > > or another service runs with elevated privileges and opens input and
> > > DRM devices on behalf of the display server.)
> >
> > So the rules are (if I'm not making a mistake)
> > - If you're not CAP_SYS_ADMIN you can't get/drop_master.
>
> Hi,
>
> not able to drop, yikes. So if someone pokes the Wayland DRM leasing
> interface while the display server is VT switched away (does not have
> DRM master), and maybe no-one else has DRM master either (you're
> hacking in VT text mode), then a new DRM fd would be master with no way
> out?
>
> So Wayland display servers should make sure they have master themselves
> before sending a supposedly non-master DRM fd to anyone else. I wonder
> if the Wayland protocol extension needs to consider that the compositor
> might not be able to send any fd soon. Being able to defer sending the
> fd should probably be mentioned in the protocol spec, so that clients
> do not expect a simple roundtrip to be enough to ensure the fd has
> arrived.
>
> > - This is a bit awkward, since non-root can become a master, when
> > there's no other master right at this point. So if you want to be able
> > to do this, we should probably clarify this part of the uapi somehow
> > (either de-priv drop_master or make sure non-root can't become master,
> > but the latter probably will break something somewhere). Plus igts to
> > lock down this behaviour. Note that if logind does a vt switch there's
> > a race window where no one is master and you might be able to squeeze
> > in. So perhaps we do want to stop this behaviour and require
> > CAP_SYS_ADMIN to become master, even accidentally.
>
> That would close the loophole that Ville mentioned, too, otherwise
> distributions should aim to not give permissions to open the DRM device
> node.

I'm kinda wondering whether we have to do this as a security fix, with
maybe a module option to get the old behaviour back for those who
need/want that. But I don't even know whom/where to ping for logind
folks ...

> > - I thought you can always re-open your own fd through proc? Which
> > should be good enough for this use-case here.
>
> We can? And that creates a new file description the same way as open()
> in the original device node?

I dreamed, it's just a normal symlink, nothing fancy.

> Does that avoid becoming master in the above VT-switched-away scenario?

Would be a reopen like open(3), so same problem until we fix that.
-Daniel

> > - Non-master primary node should indeed give you all the GET* ioctls
> > for kms, and nothing else useful or at least dangerous (you might be
> > able to render with that thing). Just make sure you dont authenticate
> > that new fd. Again maybe we should clarify our docs a bit to make this
> > use case official.
>
> Awesome, thanks,
> pq



-- 
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

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 5344 bytes --]

On Fri, 18 Oct 2019 17:47:49 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> On Fri, Oct 18, 2019 at 4:34 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > On Fri, 18 Oct 2019 16:19:33 +0200
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >  
> > > On Fri, Oct 18, 2019 at 3:43 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:  
> > > >
> > > > On Fri, 18 Oct 2019 07:54:50 -0400
> > > > "Drew DeVault" <sir@cmpwn.com> wrote:
> > > >  
> > > > > On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:  
> > > > > > One thing I did not know the last time was that apparently
> > > > > > systemd-logind may not like to give out non-master DRM fds. That might
> > > > > > need fixing in logind implementations. I hope someone would step up to
> > > > > > look into that.
> > > > > >
> > > > > > This protocol aims to deliver a harmless "read-only" DRM device file
> > > > > > description to a client, so that the client can enumerate all DRM
> > > > > > resources, fetch EDID and other properties to be able to decide which
> > > > > > connector it would want to lease. The client should not be able to
> > > > > > change any KMS state through this fd, and it should not be able to e.g.
> > > > > > spy on display contents. The assumption is that a non-master DRM fd
> > > > > > from a fresh open() would be fine for this, but is it?  
> > > > >
> > > > > What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
> > > > > path to the device file, and then I open() it and use
> > > > > drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
> > > > > to work correctly in practice.  
> > > >
> > > > That is nice.
> > > >
> > > > Personally I'm specifically worried about a setup where the user has no
> > > > access permissions to open the DRM device node directly, as is (or
> > > > should be) the case with input devices.
> > > >
> > > > However, since DRM has the master concept which input devices do not,
> > > > maybe there is no reason to prevent a normal user from opening a DRM
> > > > device directly. That is, if our assumption that a non-master DRM fd is
> > > > harmless holds.
> > > >
> > > > (Wayland display servers usually run as a normal user, while logind
> > > > or another service runs with elevated privileges and opens input and
> > > > DRM devices on behalf of the display server.)  
> > >
> > > So the rules are (if I'm not making a mistake)
> > > - If you're not CAP_SYS_ADMIN you can't get/drop_master.  
> >
> > Hi,
> >
> > not able to drop, yikes. So if someone pokes the Wayland DRM leasing
> > interface while the display server is VT switched away (does not have
> > DRM master), and maybe no-one else has DRM master either (you're
> > hacking in VT text mode), then a new DRM fd would be master with no way
> > out?
> >
> > So Wayland display servers should make sure they have master themselves
> > before sending a supposedly non-master DRM fd to anyone else. I wonder
> > if the Wayland protocol extension needs to consider that the compositor
> > might not be able to send any fd soon. Being able to defer sending the
> > fd should probably be mentioned in the protocol spec, so that clients
> > do not expect a simple roundtrip to be enough to ensure the fd has
> > arrived.
> >  
> > > - This is a bit awkward, since non-root can become a master, when
> > > there's no other master right at this point. So if you want to be able
> > > to do this, we should probably clarify this part of the uapi somehow
> > > (either de-priv drop_master or make sure non-root can't become master,
> > > but the latter probably will break something somewhere). Plus igts to
> > > lock down this behaviour. Note that if logind does a vt switch there's
> > > a race window where no one is master and you might be able to squeeze
> > > in. So perhaps we do want to stop this behaviour and require
> > > CAP_SYS_ADMIN to become master, even accidentally.  
> >
> > That would close the loophole that Ville mentioned, too, otherwise
> > distributions should aim to not give permissions to open the DRM device
> > node.  
> 
> I'm kinda wondering whether we have to do this as a security fix, with
> maybe a module option to get the old behaviour back for those who
> need/want that. But I don't even know whom/where to ping for logind
> folks ...

I would guess https://lists.freedesktop.org/mailman/listinfo/systemd-devel

It's fairly low traffic nowadays.

> > > - I thought you can always re-open your own fd through proc? Which
> > > should be good enough for this use-case here.  
> >
> > We can? And that creates a new file description the same way as open()
> > in the original device node?  
> 
> I dreamed, it's just a normal symlink, nothing fancy.

D'oh.

So we have two options: ensure logind is happy to deliver also
non-master DRM fds, or prohibit an open() on a DRM device node from
becoming master without CAP_SYS_ADMIN or something, right?

Drew does have a point though: if a non-CAP_SYS_ADMIN process gains DRM
master, it has no way to drop master, does it? Which means it's
impossible to VT-switch away from it and have something else gain
master?

Still, I can see how someone could rely on gaining master via open() and
as non-root.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Daniel Vetter]

On Mon, Oct 21, 2019 at 10:48 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Fri, 18 Oct 2019 17:47:49 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > On Fri, Oct 18, 2019 at 4:34 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > >
> > > On Fri, 18 Oct 2019 16:19:33 +0200
> > > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > > On Fri, Oct 18, 2019 at 3:43 PM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > > > >
> > > > > On Fri, 18 Oct 2019 07:54:50 -0400
> > > > > "Drew DeVault" <sir@cmpwn.com> wrote:
> > > > >
> > > > > > On Fri Oct 18, 2019 at 12:21 PM Pekka Paalanen wrote:
> > > > > > > One thing I did not know the last time was that apparently
> > > > > > > systemd-logind may not like to give out non-master DRM fds. That might
> > > > > > > need fixing in logind implementations. I hope someone would step up to
> > > > > > > look into that.
> > > > > > >
> > > > > > > This protocol aims to deliver a harmless "read-only" DRM device file
> > > > > > > description to a client, so that the client can enumerate all DRM
> > > > > > > resources, fetch EDID and other properties to be able to decide which
> > > > > > > connector it would want to lease. The client should not be able to
> > > > > > > change any KMS state through this fd, and it should not be able to e.g.
> > > > > > > spy on display contents. The assumption is that a non-master DRM fd
> > > > > > > from a fresh open() would be fine for this, but is it?
> > > > > >
> > > > > > What I do for wlroots is call drmGetDeviceNameFromFd2, which returns the
> > > > > > path to the device file, and then I open() it and use
> > > > > > drmIsMaster/drmDropMaster to make sure it's not a master fd. This seems
> > > > > > to work correctly in practice.
> > > > >
> > > > > That is nice.
> > > > >
> > > > > Personally I'm specifically worried about a setup where the user has no
> > > > > access permissions to open the DRM device node directly, as is (or
> > > > > should be) the case with input devices.
> > > > >
> > > > > However, since DRM has the master concept which input devices do not,
> > > > > maybe there is no reason to prevent a normal user from opening a DRM
> > > > > device directly. That is, if our assumption that a non-master DRM fd is
> > > > > harmless holds.
> > > > >
> > > > > (Wayland display servers usually run as a normal user, while logind
> > > > > or another service runs with elevated privileges and opens input and
> > > > > DRM devices on behalf of the display server.)
> > > >
> > > > So the rules are (if I'm not making a mistake)
> > > > - If you're not CAP_SYS_ADMIN you can't get/drop_master.
> > >
> > > Hi,
> > >
> > > not able to drop, yikes. So if someone pokes the Wayland DRM leasing
> > > interface while the display server is VT switched away (does not have
> > > DRM master), and maybe no-one else has DRM master either (you're
> > > hacking in VT text mode), then a new DRM fd would be master with no way
> > > out?
> > >
> > > So Wayland display servers should make sure they have master themselves
> > > before sending a supposedly non-master DRM fd to anyone else. I wonder
> > > if the Wayland protocol extension needs to consider that the compositor
> > > might not be able to send any fd soon. Being able to defer sending the
> > > fd should probably be mentioned in the protocol spec, so that clients
> > > do not expect a simple roundtrip to be enough to ensure the fd has
> > > arrived.
> > >
> > > > - This is a bit awkward, since non-root can become a master, when
> > > > there's no other master right at this point. So if you want to be able
> > > > to do this, we should probably clarify this part of the uapi somehow
> > > > (either de-priv drop_master or make sure non-root can't become master,
> > > > but the latter probably will break something somewhere). Plus igts to
> > > > lock down this behaviour. Note that if logind does a vt switch there's
> > > > a race window where no one is master and you might be able to squeeze
> > > > in. So perhaps we do want to stop this behaviour and require
> > > > CAP_SYS_ADMIN to become master, even accidentally.
> > >
> > > That would close the loophole that Ville mentioned, too, otherwise
> > > distributions should aim to not give permissions to open the DRM device
> > > node.
> >
> > I'm kinda wondering whether we have to do this as a security fix, with
> > maybe a module option to get the old behaviour back for those who
> > need/want that. But I don't even know whom/where to ping for logind
> > folks ...
>
> I would guess https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
> It's fairly low traffic nowadays.

Hm I thought the moved all over to github. Adding to cc, in case that
still gets somewhere.

Super short summary: While vt-switching there's a race window where
anyone who can open the primary drm fd can become drm master, even if
they're non-root. And the only way to fix up that mess is with close()
(since the dropmaster ioctl is root-only).

> > > > - I thought you can always re-open your own fd through proc? Which
> > > > should be good enough for this use-case here.
> > >
> > > We can? And that creates a new file description the same way as open()
> > > in the original device node?
> >
> > I dreamed, it's just a normal symlink, nothing fancy.
>
> D'oh.
>
> So we have two options: ensure logind is happy to deliver also
> non-master DRM fds, or prohibit an open() on a DRM device node from
> becoming master without CAP_SYS_ADMIN or something, right?
>
> Drew does have a point though: if a non-CAP_SYS_ADMIN process gains DRM
> master, it has no way to drop master, does it? Which means it's
> impossible to VT-switch away from it and have something else gain
> master?

Yeah I think it breaks vt-switching pretty bad.

> Still, I can see how someone could rely on gaining master via open() and
> as non-root.

Yeah it's the "started some program from the console/ssh with nothing
else running" use case.
-Daniel
-- 
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

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Drew DeVault]

So, I'm not sure what the action items are here. It seems like we might
have uncovered a potentially icky race condition in Linux, but that this
protocol is not really affected.
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Drew DeVault]

So, I'm not sure what the action items are here. It seems like we might
have uncovered a potentially icky race condition in Linux, but that this
protocol is not really affected.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Drew DeVault]

On Thu Oct 24, 2019 at 10:50 AM Drew DeVault wrote:
> So, I'm not sure what the action items are here. It seems like we might
> have uncovered a potentially icky race condition in Linux, but that this
> protocol is not really affected.

Bump. Happy halloween!
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v7] unstable/drm-lease: DRM lease protocol support

[Drew DeVault]

On Thu Oct 24, 2019 at 10:50 AM Drew DeVault wrote:
> So, I'm not sure what the action items are here. It seems like we might
> have uncovered a potentially icky race condition in Linux, but that this
> protocol is not really affected.

Bump. Happy halloween!
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel