GNOME Bugzilla – Bug 705861
Add support for running on bare metal
Last modified: 2013-08-30 08:34:15 UTC
A number of patches I had to run with a simple "mutter-launch -- mutter --nested" from a VT. Note that in reality, mutter won't start just with that, because glibc clears LD_LIBRARY_PATH for setuid programs, so mutter doesn't find the jhbuild libraries (we don't have .la files, so we don't get rpath). I locally patched mutter-launch to remove the check for mutter or gnome-shell, so that I could run mutter-launch -- "LD_LIBRARY_PATH=... mutter" --nested, but that's a big security bug with the current launcher, so it's not included. Also, according to the wayland BOFs, weston-launch will soon grow some of the features in this bug, so this is mainly to test stuff until that lands (or in case it misses the release)
Created attachment 251385 [details] [review] wayland: add TTY and DRM master management through weston-launch To run mutter as a display server, one needs to acquire and release the DRM master, which is only possible for root, so we take advantage of weston-launch, a small setuid helper binary written for the weston project. We import our own slightly modified copy of it, because weston-launch only launches weston, for security reasons.
Created attachment 251386 [details] [review] wayland: add input device handling too Use the new hook in clutter-evdev to ask weston-launch for the FDs of the input devices we need.
Created attachment 251387 [details] [review] Improve handling while VT switched When we don't have the DRM master, there is absolutely nothing we can do (no event processing, no video output), so emulate the old X DRM lock with a nested GMainContext without sources.
Created attachment 251388 [details] [review] Add keybindings for switching VTs Once mutter is started from weston-launch on its own VT, there is no way to change VT again (for example to actually start an application), because the keyboard is put in raw mode. So introduce some keybindings mimicking the standard X ones (Ctrl+Alt+Fn) that switch the VT manually when activated.
Created attachment 251389 [details] [review] Fix handling SIGTERM while DRM-locked And at the same time, clean up the signal handling in the regular case.
Created attachment 251390 [details] [review] Add a crash handler to restore the TTY and keyboard mode If mutter crashes on secondary VT, it leaves you with a raw keyboard that doesn't switch with Alt+FN and no way to get out. At least, let's provide a decent error message that we crash and let's restore everything to sane defaults.
Created attachment 251391 [details] [review] MetaWayland: redirect stdin/stdout/stderr when running on bare metal We need to spawn background processes for gnome-session and friends, so we need to make sure they don't try to read or write from our terminal.
We probably need this for 3.10, since we want to support running on bare metal.
Created attachment 252224 [details] [review] wayland: import weston-launch setuid launcher To run mutter as a display server, one needs to acquire and release the DRM master, which is only possible for root, so we take advantage of weston-launch, a small setuid helper binary written for the weston project. We import our own slightly modified copy of it, because weston-launch only launches weston, for security reasons.
Created attachment 252225 [details] [review] mutter-launch: simplify by removing features we don't need Remove the ability to launch as a different user, which we don't need because we're spawned by gdm or by the user manually on the command line. With that, we can also remove "security" checks.
Created attachment 252226 [details] [review] mutter-launch: make sure that the spawned binaries sees the right libraries Being a setuid binary, our LD_LIBRARY_PATH is cleared by glibc at startup, but we need the spawned binary to see it, otherwise jhbuild doesn't work, so hardcode it using the configured libdir.
Created attachment 252227 [details] [review] wayland: add TTY and DRM master management Now that we have a setuid launcher binary, we can make use of using the standard weston-launch protocol, through the socket we're passed at startup. We also add TTY management, to ensure the keyboard is in the correct mode and the VT switching is in our control.
Created attachment 252228 [details] [review] wayland: add input device handling through mutter-launch Use the new hook in clutter-evdev to ask mutter-launch for the FDs of the input devices we need.
Review of attachment 252224 [details] [review]: Gross =( In gnome-ostree, I dropped the setuid bit from /usr/bin/Xorg; the only way to launch X is via gdm. Why can't we have a model where GDM allows suitably authorized callers to allocate a mutter server run as root? It could even allow it by default for uids that are already part of a session.
Or alternatively, do the work of creating a DBus service (or reuse gdm) that opens the DRM and input devices on behalf of callers and can pass the fd to them (if authorized via polkit), then just run mutter as a regular user.
(In reply to comment #15) > Or alternatively, do the work of creating a DBus service (or reuse gdm) that > opens the DRM and input devices on behalf of callers and can pass the fd to > them (if authorized via polkit), then just run mutter as a regular user. This will be logind, as it already owns all the devices and handing them out will be a piece of cake, according to Lennart/Kay. This is for the short-term where we don't have that yet. We agreed at GUADEC at the Wayland BoF that for the tech preview we don't want to involve gdm, and that users should log into a VT and type 'mutter-launch' themselves.
(In reply to comment #16) > > This will be logind, as it already owns all the devices and handing them out > will be a piece of cake, according to Lennart/Kay. This is for the short-term > where we don't have that yet. If it's so easy why are we not writing that code and depending on it instead of adding a lot of security-sensitive workaround code? > We agreed at GUADEC at the Wayland BoF Ok. I wasn't there obviously, so feel free to just go ahead, since I understand the value of getting a tech preview out so we can iterate on fixing higher level things.
Created attachment 252450 [details] [review] mutter-launch: support spawning with stdin to /dev/null If we don't have stdin connected to a terminal, and we don't see the -t argument, assume the terminal we want is $XDG_VTNR
Created attachment 252711 [details] [review] mutter-launch: use systemd to obtain the TTY Using the command line or an environment variable is dangerous, as those can be spoofed to gain access to other sessions.
Created attachment 252728 [details] [review] mutter-launch: augment with VT and TTY handling Set the TTY mode appropriately at startup, and clean it up when the compositor exits. Also, take control of VT switching, and make sure that the compositor calls drmDropMaster when switched away. In the future, we the kernel implements the mute evdev ioctl, we'll also make sure that input devices are appropriately released.
Created attachment 252729 [details] [review] wayland: use the new mutter-launch capabilities to handle TTY and VT Integrate MetaTTY with meta-weston-launch, into a new MetaLauncher helper, that takes care of talking with the setuid helper, calling set/dropMaster as appropriate and opening the evdev devices when asked by clutter. In the future, this object could be made to talk to logind instead of our private helper, or the helper could be turned into a systemd daemon.
Created attachment 252900 [details] [review] Add keybindings for switching VTs Once mutter is started from weston-launch on its own VT, there is no way to change VT again (for example to actually start an application), because the keyboard is put in raw mode. So introduce some keybindings mimicking the standard X ones (Ctrl+Alt+Fn) that switch the VT manually when activated. Amended to use a different schema (org.gnome.mutter.wayland), so that we don't crash when by chance we load the schema from the master branch.
Kristian, you were mentioned as a good candidate for reviewing this patch series.
Review of attachment 251387 [details] [review]: Looks good. There's a bunch of different fixes in the patch (the env_get_fd() fix, the meta-weston-launch stuff), but I don't know how strict you want to be about splitting it out. ::: src/wayland/meta-tty.c @@ +105,3 @@ + have input devices and we don't have the DRM master, + so let's run a nested busy loop until the VT is reentered */ + g_main_loop_run (tty->nested_loop); That makes sense, good idea. Assuming that we close input devices (and reopen on vt enter, which we need for logind integration anyway).
Review of attachment 251389 [details] [review]: main.c changes looks like a nice, straightforward cleanup, but I'm not sure how the SIGTERM handling in the nested loop is supposed to work. ::: src/wayland/meta-tty.c @@ +309,3 @@ + tty->nested_term = g_unix_signal_source_new (SIGTERM); + g_source_set_callback (tty->nested_term, quit_nested_loop, tty, NULL); + Does SIGTERM now just exit the lock loop? Is there other SIGTERM handling elsewhere? As far as I understand, SIGTERM while locked will try to re-acquire the VT and set master, which wont work if you send the signal from another VT.
Review of attachment 251390 [details] [review]: Patch looks fine, but medium term we'll move this to the launcher and longer term to logind (or maybe just skip straight to logind, David Herrmann is making good progress: https://plus.google.com/u/0/112212087950959620804/posts/K9XdeZqvVSX). If you take a segfault, your process state is undefined and you can't reliably clean up and restore tty. Letting the launcher/logind do it is more reilable.
Review of attachment 251391 [details] [review]: If you're launched from systemd stdout/err will go into the systemd journal, which is preferred to a growing log file in the user home dir. Not sure how it will work when launched from gdm, but I think the goal should be to log to the journal.
Review of attachment 252224 [details] [review]: Colin, that can't work. And what's gross about this? Any display server under Linux needs to run as root or have a root-helper because it has to drop and set "drm master" to properly vt switch and it has to open root-owned input device (at startup and later in cases of hotplug). We're not going to run mutter as root. weston-launch / mutter-launch is much like startx for X and I know that that doesn't fit into the "GNOME Vision" of how the desktop should be started, but there's more than a few valid use-cases outside that world view. I do agree that for something like the GNOME desktop, we want to not use mutter-launch, but that's already in the works: https://plus.google.com/u/0/112212087950959620804/posts/K9XdeZqvVSX with Davids logind work, logind will understand graphical sessions better and will be able to grant and revoke drm master (for kms access) and input device access, as well as setting up and restoring the VT for a graphical server. For this patch: importing weston-launch makes sense to get mutter off the ground, and I think that it may be fine to just stick with this until we can start using logind - that is, I don't think we need to try to sync up between weston-launch and mutter-launch. Just make mutter-launch work for mutter, and drop it once we can start using logind.
Review of attachment 252225 [details] [review]: You can mock the "security" all you want, it's there for a reason. The intention is for weston-launch to be allowed if-and-only-if the user would otherwise be able to just open /dev/dri/card0 and start using KMS. That is, whenever udev and logind have set the facls on the device file, which is when you have an active, local session (or group membership for systemd-challenged distros). In particular, you can't log in remotely and start weston-launch. Removing the run-as-other user feature and all the pam junk makes a lot of sense, but removing the is_allowed() check changes the intended operation of weston-launch. Now, you may want to change how it's supposed to work, which is fine, it's a temporary band-aid anyway, but now you know why it's there.
(In reply to comment #29) > Review of attachment 252224 [details] [review]: > > Colin, that can't work. And what's gross about this? Any display server under > Linux needs to run as root Do not conflate running as root with being setuid root. They're totally different things. I think *setuid root* binaries are something we should avoid as much as possible. If you read my comment, I suggested having gdm launch mutter *as root* in the same way it's launching the X server. Again, gnome-ostree ships without a setuid /usr/bin/Xorg and it works fine; you can only log in through GDM, and I don't care about startx.
(In reply to comment #28) > Review of attachment 252224 [details] [review]: > weston-launch / mutter-launch is much like startx for X and I know that that > doesn't fit into the "GNOME Vision" of how the desktop should be started, but > there's more than a few valid use-cases outside that world view. What valid use cases?
Review of attachment 252226 [details] [review]: As a temp workaround this is OK, I don't see a good solution to this as long as we're going to mutter through the setuid binary. With logind mutter can just run as the user and talk to logind so this problem will go away. Once we have EVIOCREVOKE (http://www.spinics.net/lists/linux-input/msg27175.html), we could make weston-launch a setuid helper that is launched by the compositor in case logind is not available. weston-launch will then revoke input devices on vt switch and can be a general purpose helper for use by any display server (standalone kms/evdev apps). In that model, LD_LIBRARY_PATH is preserved for the display server and this problem is moot. For mutter, you may just want to only focus on logind of course.
(In reply to comment #30) > Review of attachment 252225 [details] [review]: > > You can mock the "security" all you want, it's there for a reason. The > intention is for weston-launch to be allowed if-and-only-if the user would > otherwise be able to just open /dev/dri/card0 and start using KMS. That is, > whenever udev and logind have set the facls on the device file, which is when > you have an active, local session (or group membership for systemd-challenged > distros). In particular, you can't log in remotely and start weston-launch. > Removing the run-as-other user feature and all the pam junk makes a lot of > sense, but removing the is_allowed() check changes the intended operation of > weston-launch. > > Now, you may want to change how it's supposed to work, which is fine, it's a > temporary band-aid anyway, but now you know why it's there. Heh... you can't run weston-launch from ssh... unless you're in the magic group. At which point, is like saying everyone is allowed (if every user should be in the group), or only root is allowed (because the group gives you root-like privileges). The final patch (in branch order, not bugzilla order) has a better security check against the ssh launch case, I think. Also, I feel me, you and Colin are a bit confused with the purpose of mutter-launch at this point, as well as how stuff is supposed to work in the future. My plan was to keep using mutter-launch only until logind has the new API (which won't make 3.10 at this point), and then kill it. I don't support startx for GNOME at all, although I'm working to make it possible to run gnome-session from a VT, configured with a wayland session. In this model, which I plan to keep after 3.10, GDM does not launch mutter/mutter-launch by itself (or worse, mutter as root!), it simply skips launching the X server for wayland sessions, and launches gnome-session with the user uid.
Review of attachment 252227 [details] [review]: Ugh, I'm not sure which order to review these patches anymore... is there a branch somewhere instead? Anyway, it'd be nice to fix the get_env_fd bug() in this patch instead of a separate patch, and in general, I think it would work better and be easier to review if we just land the final state of the mutter-launch / meta-tty.c / etc instead of landing something and then rewriting it in a handful follow-up patches.
(In reply to comment #36) > > My plan was to keep using mutter-launch only until logind has the new API > (which won't make 3.10 at this point), and then kill it. That makes sense to me.
(In reply to comment #33) > (In reply to comment #28) > > Review of attachment 252224 [details] [review] [details]: > > > weston-launch / mutter-launch is much like startx for X and I know that that > > doesn't fit into the "GNOME Vision" of how the desktop should be started, but > > there's more than a few valid use-cases outside that world view. > > What valid use cases? In case of weston, we need to launch it from a systemd script. Much like autologin, but without gdm in the loop at all. Like how your phone or car doesn't boot into a user/password login screen.
(In reply to comment #39) > (In reply to comment #33) > > (In reply to comment #28) > > > Review of attachment 252224 [details] [review] [details] [details]: > > > > > weston-launch / mutter-launch is much like startx for X and I know that that > > > doesn't fit into the "GNOME Vision" of how the desktop should be started, but > > > there's more than a few valid use-cases outside that world view. > > > > What valid use cases? > > In case of weston, we need to launch it from a systemd script. Much like > autologin, but without gdm in the loop at all. Like how your phone or car > doesn't boot into a user/password login screen. But yes, when running from a systemd script, you're running as root so you don't need setuid in that case. Launching from a VT for development is a use case, but you could use sudo for that if it's just development, I suppose. Anyway, maybe I got a little defensive, but it's not clear from the "Gross =(" comment which part you object to or why. And while weston-launch may seem like an awful hack to you, it's been an important intermediate result in the process of moving from "display server running as root and linking to libGL and talking to clients" to "display server runs as the user, logind handles the privileged stuff". It's only recently that we got consensus on moving the support into logind and David Herrmann has been spending most of his GSoC on designing and implementing the new interfaces. So it's not exactly a piece of cake as Jasper says and certainly not something that we can realize for the F20 snapshot.
Created attachment 253352 [details] [review] wayland: import weston-launch setuid launcher To run mutter as a display server, one needs to acquire and release the DRM master, which is only possible for root, so we take advantage of weston-launch, a small setuid helper binary written for the weston project. We import our own slightly modified copy of it, because weston-launch only launches weston, for security reasons.
Created attachment 253353 [details] [review] mutter-launch: simplify by removing features we don't need Remove the ability to launch as a different user, which we don't need because we're spawned by gdm or by the user manually on the command line. At the same time, require an active local session, and remove the ability to run from anywhere by being in the right user group (which automatically gives you root-like privileges)
Created attachment 253354 [details] [review] mutter-launch: make sure that the spawned binaries sees the right libraries Being a setuid binary, our LD_LIBRARY_PATH is cleared by glibc at startup, but we need the spawned binary to see it, otherwise jhbuild doesn't work, so hardcode it using the configured libdir.
Created attachment 253355 [details] [review] mutter-launch: use systemd to obtain the TTY Using the command line or an environment variable is dangerous, as those can be spoofed to gain access to other sessions.
Created attachment 253356 [details] [review] mutter-launch: augment with VT and TTY handling Set the TTY mode appropriately at startup, and clean it up when the compositor exits. Also, take control of VT switching, and make sure that the compositor calls drmDropMaster when switched away. In the future, we the kernel implements the mute evdev ioctl, we'll also make sure that input devices are appropriately released.
Created attachment 253357 [details] [review] wayland: add TTY and DRM master management Now that we have a setuid launcher binary, we can make use of using a private protocol through the socket we're passed at startup. We also use the new hook in clutter-evdev to ask mutter-launch for the FDs of the input devices we need, and we emulate the old X DRM lock with a nested GMainContext without sources. In the future, mutter-launch will be replaced with the new logind API currently in development.
Created attachment 253358 [details] [review] Add keybindings for switching VTs Once mutter is started from weston-launch on its own VT, there is no way to change VT again (for example to actually start an application), because the keyboard is put in raw mode. So introduce some keybindings mimicking the standard X ones (Ctrl+Alt+Fn) that switch the VT manually when activated.
(In reply to comment #40) > Anyway, maybe I got a little defensive, but it's not clear from the "Gross =(" > comment which part you object to or why. I really dislike setuid binaries; now while I admit it's not practical yet to ship a general purpose system without them, I'd really like to not add new complex ones. > It's only recently that we got consensus on moving the support into > logind and David Herrmann has been spending most of his GSoC on designing and > implementing the new interfaces. So it's not exactly a piece of cake as Jasper > says and certainly not something that we can realize for the F20 snapshot. I agree with this!
Review of attachment 253352 [details] [review]: Looks good.
Review of attachment 253353 [details] [review]: Yup, this looks good now, definitely remove the group check. However, I don't see how being in the group gives you root-like priviledges, it just lets you run weston-launch.
Review of attachment 253354 [details] [review]: Yeah, this one is fine still.
Review of attachment 253355 [details] [review]: That's a good change, makese sense.
Review of attachment 253356 [details] [review]: This looks good. Moving the tty handling and reset to mutter-launch makes crash recovery a lot more robust. logind will require you to open the drm device using the equivalent of WESTON_LAUNCHER_OPEN and then handle drm drop master internally after receiving the VT switch ack and set master before sending the WESTON_LAUNCHER_ACTIVATE_VT message. I suspect cogl limitation means we can't quite do that yet. I still think it would be better if we replace WESTON_LAUNCHER_DRM_SET_MASTER with a request to pass the drm fd (WAYLAND_LAUNCHER_SET_DRM_FD) to the launcher at startup and then make the launcher handle drop/set master. There's less that can go wrong that way and the interface is a little closer to what it will be with logind. If cogl can actually initialize on a given fd, we should do that instead and use WESTON_LAUNCHER_OPEN to open the device. The launcher will verify the drm fd belongs to the seat we're running on and that it's a drm device (using libudev) and then keep the fd around for drop/set master.
Review of attachment 253357 [details] [review]: This all looks good, modulo comments about doing drm drop/set master in the launcher.
Review of attachment 253358 [details] [review]: Looks good to me.
(In reply to comment #50) > Review of attachment 253353 [details] [review]: > > Yup, this looks good now, definitely remove the group check. However, I don't > see how being in the group gives you root-like priviledges, it just lets you > run weston-launch. From which you can acquire the drm master and evdev devices, and start snooping on another session, including possibly a root one.
Created attachment 253486 [details] [review] mutter-launch: augment with VT and TTY handling Set the TTY mode appropriately at startup, and clean it up when the compositor exits. Also, take control of VT switching, including the calls to drmSetMaster and drmDropMaster as appropriate. In the future, we the kernel implements the mute evdev ioctl, we'll also make sure that input devices are appropriately released.
Created attachment 253487 [details] [review] wayland: add TTY and DRM master management Now that we have a setuid launcher binary, we can make use of using a private protocol through the socket we're passed at startup. We also use the new hook in clutter-evdev to ask mutter-launch for the FDs of the input devices we need, and we emulate the old X DRM lock with a nested GMainContext without sources. In the future, mutter-launch will be replaced with the new logind API currently in development.
Review of attachment 253486 [details] [review]: This looks good now.
Review of attachment 253487 [details] [review]: There's a problem when handling incoming messages from weston-launch. The send_message_to_wl() will block for a reply, but when weston-launch can send WESTON_LAUNCHER_SERVER_REQUEST_VT_SWITCH or WESTON_LAUNCHER_SERVER_VT_ENTER at any time, we may be receiving one of those while blocking for a reply.
Attachment 253352 [details] pushed as fd40a12 - wayland: import weston-launch setuid launcher Attachment 253353 [details] pushed as 2d27873 - mutter-launch: simplify by removing features we don't need Attachment 253354 [details] pushed as 1c34f0b - mutter-launch: make sure that the spawned binaries sees the right libraries Attachment 253355 [details] pushed as e263b36 - mutter-launch: use systemd to obtain the TTY Attachment 253358 [details] pushed as e311cef - Add keybindings for switching VTs Attachment 253486 [details] pushed as 96fa518 - mutter-launch: augment with VT and TTY handling Attachment 253487 [details] pushed as e72f81c - wayland: add TTY and DRM master management