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