On Fri, Oct 18, 2019 at 4:34 PM Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: > > On Fri, 18 Oct 2019 16:19:33 +0200 > Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > > > On Fri, Oct 18, 2019 at 3:43 PM Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: > > > > > > On Fri, 18 Oct 2019 07:54:50 -0400 > > > "Drew DeVault" <sir@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
