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

[Pekka Paalanen]

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

> On Fri, Oct 18, 2019 at 3:43 PM Pekka Paalanen <ppaalanen at gmail.com> wrote:
> >
> > On Fri, 18 Oct 2019 07:54:50 -0400
> > "Drew DeVault" <sir at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20191018/e4e7aa8a/attachment-0001.sig>