After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705861 - Add support for running on bare metal
Add support for running on bare metal
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
wayland
Depends on: 704269 705816
Blocks:
 
 
Reported: 2013-08-12 16:22 UTC by Giovanni Campagna
Modified: 2013-08-30 08:34 UTC
See Also:
GNOME target: 3.10
GNOME version: ---


Attachments
wayland: add TTY and DRM master management through weston-launch (43.32 KB, patch)
2013-08-12 16:23 UTC, Giovanni Campagna
none Details | Review
wayland: add input device handling too (2.86 KB, patch)
2013-08-12 16:23 UTC, Giovanni Campagna
none Details | Review
Improve handling while VT switched (7.11 KB, patch)
2013-08-12 16:23 UTC, Giovanni Campagna
none Details | Review
Add keybindings for switching VTs (8.52 KB, patch)
2013-08-12 16:23 UTC, Giovanni Campagna
none Details | Review
Fix handling SIGTERM while DRM-locked (4.69 KB, patch)
2013-08-12 16:23 UTC, Giovanni Campagna
none Details | Review
Add a crash handler to restore the TTY and keyboard mode (3.91 KB, patch)
2013-08-12 16:23 UTC, Giovanni Campagna
none Details | Review
MetaWayland: redirect stdin/stdout/stderr when running on bare metal (1.83 KB, patch)
2013-08-12 16:23 UTC, Giovanni Campagna
none Details | Review
wayland: import weston-launch setuid launcher (19.46 KB, patch)
2013-08-19 13:29 UTC, Giovanni Campagna
none Details | Review
mutter-launch: simplify by removing features we don't need (6.32 KB, patch)
2013-08-19 13:29 UTC, Giovanni Campagna
none Details | Review
mutter-launch: make sure that the spawned binaries sees the right libraries (1.51 KB, patch)
2013-08-19 13:29 UTC, Giovanni Campagna
none Details | Review
wayland: add TTY and DRM master management (24.77 KB, patch)
2013-08-19 13:29 UTC, Giovanni Campagna
none Details | Review
wayland: add input device handling through mutter-launch (2.88 KB, patch)
2013-08-19 13:30 UTC, Giovanni Campagna
none Details | Review
mutter-launch: support spawning with stdin to /dev/null (1.05 KB, patch)
2013-08-20 16:29 UTC, Giovanni Campagna
none Details | Review
mutter-launch: use systemd to obtain the TTY (4.70 KB, patch)
2013-08-22 10:09 UTC, Giovanni Campagna
none Details | Review
mutter-launch: augment with VT and TTY handling (9.38 KB, patch)
2013-08-22 12:47 UTC, Giovanni Campagna
none Details | Review
wayland: use the new mutter-launch capabilities to handle TTY and VT (35.35 KB, patch)
2013-08-22 12:47 UTC, Giovanni Campagna
none Details | Review
Add keybindings for switching VTs (9.98 KB, patch)
2013-08-23 16:04 UTC, Giovanni Campagna
none Details | Review
wayland: import weston-launch setuid launcher (19.46 KB, patch)
2013-08-28 09:14 UTC, Giovanni Campagna
committed Details | Review
mutter-launch: simplify by removing features we don't need (6.71 KB, patch)
2013-08-28 09:15 UTC, Giovanni Campagna
committed Details | Review
mutter-launch: make sure that the spawned binaries sees the right libraries (1.51 KB, patch)
2013-08-28 09:15 UTC, Giovanni Campagna
committed Details | Review
mutter-launch: use systemd to obtain the TTY (4.83 KB, patch)
2013-08-28 09:15 UTC, Giovanni Campagna
committed Details | Review
mutter-launch: augment with VT and TTY handling (9.38 KB, patch)
2013-08-28 09:15 UTC, Giovanni Campagna
none Details | Review
wayland: add TTY and DRM master management (21.22 KB, patch)
2013-08-28 09:15 UTC, Giovanni Campagna
none Details | Review
Add keybindings for switching VTs (7.58 KB, patch)
2013-08-28 09:15 UTC, Giovanni Campagna
committed Details | Review
mutter-launch: augment with VT and TTY handling (12.25 KB, patch)
2013-08-29 11:15 UTC, Giovanni Campagna
committed Details | Review
wayland: add TTY and DRM master management (20.96 KB, patch)
2013-08-29 11:15 UTC, Giovanni Campagna
committed Details | Review

Description Giovanni Campagna 2013-08-12 16:22:58 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)
Comment 1 Giovanni Campagna 2013-08-12 16:23:01 UTC
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.
Comment 2 Giovanni Campagna 2013-08-12 16:23:05 UTC
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.
Comment 3 Giovanni Campagna 2013-08-12 16:23:09 UTC
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.
Comment 4 Giovanni Campagna 2013-08-12 16:23:13 UTC
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.
Comment 5 Giovanni Campagna 2013-08-12 16:23:17 UTC
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.
Comment 6 Giovanni Campagna 2013-08-12 16:23:21 UTC
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.
Comment 7 Giovanni Campagna 2013-08-12 16:23:25 UTC
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.
Comment 8 Matthias Clasen 2013-08-17 16:02:01 UTC
We probably need this for 3.10, since we want to support running on bare metal.
Comment 9 Giovanni Campagna 2013-08-19 13:29:44 UTC
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.
Comment 10 Giovanni Campagna 2013-08-19 13:29:49 UTC
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.
Comment 11 Giovanni Campagna 2013-08-19 13:29:53 UTC
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.
Comment 12 Giovanni Campagna 2013-08-19 13:29:57 UTC
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.
Comment 13 Giovanni Campagna 2013-08-19 13:30:02 UTC
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.
Comment 14 Colin Walters 2013-08-19 14:39:25 UTC
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.
Comment 15 Colin Walters 2013-08-19 14:43:38 UTC
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.
Comment 16 Jasper St. Pierre (not reading bugmail) 2013-08-19 14:46:15 UTC
(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.
Comment 17 Colin Walters 2013-08-19 15:38:12 UTC
(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.
Comment 18 Giovanni Campagna 2013-08-20 16:29:20 UTC
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
Comment 19 Giovanni Campagna 2013-08-22 10:09:31 UTC
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.
Comment 20 Giovanni Campagna 2013-08-22 12:47:34 UTC
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.
Comment 21 Giovanni Campagna 2013-08-22 12:47:39 UTC
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.
Comment 22 Giovanni Campagna 2013-08-23 16:04:50 UTC
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.
Comment 23 Matthias Clasen 2013-08-26 16:54:11 UTC
Kristian, you were mentioned as a good candidate for reviewing this patch series.
Comment 24 Kristian Høgsberg 2013-08-27 20:27:35 UTC
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).
Comment 25 Kristian Høgsberg 2013-08-27 20:33:46 UTC
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.
Comment 26 Kristian Høgsberg 2013-08-27 20:44:18 UTC
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.
Comment 27 Kristian Høgsberg 2013-08-27 20:50:55 UTC
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.
Comment 28 Kristian Høgsberg 2013-08-27 21:01:23 UTC
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.
Comment 29 Kristian Høgsberg 2013-08-27 21:01:50 UTC
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.
Comment 30 Kristian Høgsberg 2013-08-27 21:10:14 UTC
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.
Comment 31 Kristian Høgsberg 2013-08-27 21:10:15 UTC
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.
Comment 32 Colin Walters 2013-08-27 21:10:48 UTC
(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.
Comment 33 Colin Walters 2013-08-27 21:14:21 UTC
(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?
Comment 34 Kristian Høgsberg 2013-08-27 21:18:38 UTC
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.
Comment 35 Kristian Høgsberg 2013-08-27 21:18:39 UTC
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.
Comment 36 Giovanni Campagna 2013-08-27 21:23:38 UTC
(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.
Comment 37 Kristian Høgsberg 2013-08-27 21:26:01 UTC
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.
Comment 38 Colin Walters 2013-08-27 21:38:06 UTC
(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.
Comment 39 Kristian Høgsberg 2013-08-28 04:54:05 UTC
(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.
Comment 40 Kristian Høgsberg 2013-08-28 05:13:55 UTC
(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.
Comment 41 Giovanni Campagna 2013-08-28 09:14:24 UTC
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.
Comment 42 Giovanni Campagna 2013-08-28 09:15:27 UTC
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)
Comment 43 Giovanni Campagna 2013-08-28 09:15:32 UTC
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.
Comment 44 Giovanni Campagna 2013-08-28 09:15:37 UTC
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.
Comment 45 Giovanni Campagna 2013-08-28 09:15:42 UTC
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.
Comment 46 Giovanni Campagna 2013-08-28 09:15:46 UTC
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.
Comment 47 Giovanni Campagna 2013-08-28 09:15:52 UTC
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.
Comment 48 Colin Walters 2013-08-28 16:06:10 UTC
(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!
Comment 49 Kristian Høgsberg 2013-08-28 17:50:45 UTC
Review of attachment 253352 [details] [review]:

Looks good.
Comment 50 Kristian Høgsberg 2013-08-28 17:55:46 UTC
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.
Comment 51 Kristian Høgsberg 2013-08-28 17:56:17 UTC
Review of attachment 253354 [details] [review]:

Yeah, this one is fine still.
Comment 52 Kristian Høgsberg 2013-08-28 17:58:05 UTC
Review of attachment 253355 [details] [review]:

That's a good change, makese sense.
Comment 53 Kristian Høgsberg 2013-08-28 18:25:58 UTC
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.
Comment 54 Kristian Høgsberg 2013-08-28 18:27:22 UTC
Review of attachment 253357 [details] [review]:

This all looks good, modulo comments about doing drm drop/set master in the launcher.
Comment 55 Kristian Høgsberg 2013-08-28 18:28:34 UTC
Review of attachment 253358 [details] [review]:

Looks good to me.
Comment 56 Kristian Høgsberg 2013-08-28 18:28:34 UTC
Review of attachment 253358 [details] [review]:

Looks good to me.
Comment 57 Giovanni Campagna 2013-08-29 08:42:32 UTC
(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.
Comment 58 Giovanni Campagna 2013-08-29 11:15:24 UTC
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.
Comment 59 Giovanni Campagna 2013-08-29 11:15:30 UTC
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.
Comment 60 Kristian Høgsberg 2013-08-29 21:01:50 UTC
Review of attachment 253486 [details] [review]:

This looks good now.
Comment 61 Kristian Høgsberg 2013-08-29 21:16:36 UTC
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.
Comment 62 Kristian Høgsberg 2013-08-29 21:16:37 UTC
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.
Comment 63 Giovanni Campagna 2013-08-30 08:33:38 UTC
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