[PATCH 3/7] Make weston spawn weston-launch

David Herrmann dh.herrmann at gmail.com
Wed Oct 16 14:39:26 CEST 2013


Hi

On Tue, Oct 15, 2013 at 8:07 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> On Tue, Oct 15, 2013 at 02:29:58PM +0200, David Herrmann wrote:
>> This is a rather complete rewrite of weston-launch. Instead of spawning
>> weston from weston-launch, we do the inverse now. Whenever weston is
>> spawned with a backend that uses launcher-util, we spawn weston-launch as
>> child process. weston-launch still handles VT switching and open() for
>> session-devices and passes them back to weston.
>>
>> However, this drops all PAM, --user and session handling from
>> weston-launch. If we spawn weston-launch from weston, we cannot do any PAM
>> handling as it wouldn't affect weston, anymore. But this doesn't hurt as
>> the same effects can be achieved via "su", "sudo" or "/bin/login". There
>> is no real reason to do this in weston.
>> If you want weston to run in a new session, use one of these helpers to
>> create the session and then spawn weston from within the new session.
>>
>> Some more differences are:
>>
>> (1) We have now two sockets from weston to weston-launch. One for OPEN and
>> one for EVENTS. The reason for that is that if we send an OPEN request and
>> weston-launch schedules some VT-EVENT before sending the OPEN response,
>> launcher-util needs to handle the VT-EVENT from within
>> weston_launcher_open(). This isn't very hard to do, but may cause weird
>> side-effects as the callstack might already be an EVENT handling. There
>> are other ways to handle that, but the simplest way is to have two
>> independent queues.
>
> I thought I handled this in current weston-launch by scheduling an
> idle callback to handle the event if I get an event while waiting for
> a response.  But I don't see the code there, so I must have failed to
> do that, but I talked to Giovanni about this for mutter-launch and he
> did it there.  Using two socketpairs has much the same effect and
> shouldn't be a big deal.

Problem with idle-callback is that we can schedule it only once. So
multiple events may be merged. It might work if we remember each event
individually, but I thought two pipes is the easier way.

>> (2) VT_ACTIVATE is privileged, so we pass the request to weston-launch
>> which then issues the VT_ACTIVATE ioctl.
>
> Are you sure?  I can login in text mode and use chvt to vt switch away
> from that text login.  I think if your tty is a vt and that vt is
> currently active, you're allowed to switch away.  That is, you can't
> run on an inactive VT and then vt switch to your VT, but you're
> allowed to switch away.  Which is all weston needs and it's how
> launcher-util.c currently does it.  As for the initial VT switch to
> the weston VT, that is done in weston-launch.
>
> Actually, I just tried to log in on vt2, do a "sleep 2; chvt 3" and
> then switch to vt 1.  After the two seconds I'm switched back to vt 2.
> chvt isn't setuid and I run is as user krh.  It fails under X because
> it tries to open /prof/self/fd/0, which is the pty of your X terminal.
>
> But if I try
>
>   [krh at tokamak weston]$ chvt 2 0</dev/tty1
>   chvt: VT_ACTIVATE: Operation not permitted
>
> so I'm not really sure what the rules are.
>
> Mainly, I'd just like to not have this in the weston-launch protocol.
> For logind it doesn't matter since we use session.Activate there, but
> if we can keep the weston-launch case simpler by not having more
> protocol than we need to, that's better.

VT ioctls are allowed by:
 - CAP_SYS_TTY_CONFIG
 - processes with the VT as controlling-tty

Problem with the latter is that only a single session can have a given
TTY as controlling TTY. Overwriting this requires root. So if you
start weston from within a shell on a VT, you cannot call
setsid()+TIOCSCTTY from weston, as the shell still has the VT as
controlling-TTY...

That's why I thought the easiest way is to let weston-launch do that as root.

>> (3) weston-launch is mandatory now. Every backend which uses launcher-util
>> requires weston-launch now. However, setuid is *not* mandatory! If
>> weston-launch does not have the setuid bit, you have to run weston as root
>> (as usual).
>
> That's fine, the reason I kept the ability to run without
> weston-launch was that having to run weston through weston-launch gets
> in the way of debugging and testing.  Now that we're running weston
> directly even when it's using weston-launch, we can drop that code
> path.

Ok, Benjamin's comment actually made me reconsider this. If
weston-launch is spawned by weston, any other session may also use
weston-launch. I doubt there's a safe way for us to verify we're
called from weston and not from a random attacker.

Attack-scenarios would be:
Attacker spawns weston-launch, retrieves all input file-descriptors
and then kills weston-launch. We currently do not track FDs in
weston-launch so we cannot revoke them during exit(). The attacker can
now listen on the input-FDs if any other compositor starts on the VT.

Ideas:
 - require root or weston-launch group to spawn weston-launch. (that
is, we drop the systemd-session check)
 - keep the current style by spawning weston from weston-launch
 - improving weston-launch to close FDs during exit() (which would
also require WESTON_LAUNCH_CLOSE commands)

I am kind of dismotivated to improve weston-launch to basically do
what logind does. It's a huge amount of work and only useful as
fallback. I'd rather spend the time on improving logind.

Furthermore, weston-launch currently lacks seat-awareness. So a user
can basically open _any_ input device via weston-launch even from
other seats.

We can fix all that, but maybe it's easier to keep the current style
and just drop this patch. Ideas welcome..

>> (4) Reading VTNR from systemd is now supported (requires systemd-207,
>> actually without the unreleased systemd-209 it will segfault due to a bug
>> in the implementation of sd_session_get_vt())
>>
>> (5) I pass -v by default to weston-launch now. If someone doesn't like
>> that kind of verbosity, feel free to add a new command-line argument to
>> weston which forwards this flag.
>
> There are a few questions below, but over all, this looks good.  I
> almost applied this - the two things below we could fix up later, but
> I'd like find out what's up with VT_ACTIVATE.
>
> Kristian
>
>> ---
>>  configure.ac        |  30 ++--
>>  src/Makefile.am     |  38 ++---
>>  src/launcher-util.c | 430 ++++++++++++++++++++++++++++++----------------------
>>  src/weston-launch.c | 401 +++++++++++++-----------------------------------
>>  src/weston-launch.h |   6 +
>>  5 files changed, 386 insertions(+), 519 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 950086d..93a38ac 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -53,7 +53,7 @@ AC_CHECK_DECL(CLOCK_MONOTONIC,[],
>>             [[#include <time.h>]])
>>  AC_CHECK_HEADERS([execinfo.h])
>>
>> -AC_CHECK_FUNCS([mkostemp strchrnul initgroups])
>> +AC_CHECK_FUNCS([mkostemp strchrnul])
>>
>>  COMPOSITOR_MODULES="wayland-server >= 1.2.91 pixman-1"
>>
>> @@ -316,21 +316,18 @@ AC_ARG_ENABLE(resize-optimization,
>>  AS_IF([test "x$enable_resize_optimization" = "xyes"],
>>        [AC_DEFINE([USE_RESIZE_POOL], [1], [Use resize memory pool as a performance optimization])])
>>
>> -AC_ARG_ENABLE(weston-launch, [  --enable-weston-launch],, enable_weston_launch=yes)
>> -AM_CONDITIONAL(BUILD_WESTON_LAUNCH, test x$enable_weston_launch == xyes)
>> -if test x$enable_weston_launch == xyes; then
>> -  PKG_CHECK_MODULES(WESTON_LAUNCH, [libdrm])
>> -  PKG_CHECK_MODULES(SYSTEMD_LOGIN, [libsystemd-login],
>> -                 [have_systemd_login=yes], [have_systemd_login=no])
>> -  AS_IF([test "x$have_systemd_login" = "xyes"],
>> -     [AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])])
>> -
>> -  AC_CHECK_LIB([pam], [pam_open_session], [have_pam=yes], [have_pam=no])
>> -  if test x$have_pam == xno; then
>> -    AC_ERROR([weston-launch requires pam])
>> -  fi
>> -  WESTON_LAUNCH_LIBS="$WESTON_LAUNCH_LIBS -lpam"
>> -fi
>> +PKG_CHECK_MODULES(SYSTEMD_LOGIN, [libsystemd-login],
>> +                  [have_systemd_login=yes], [have_systemd_login=no])
>> +AS_IF([test "x$have_systemd_login" = "xyes"],
>> +      [AC_DEFINE([HAVE_SYSTEMD_LOGIN], [1], [Have systemd-login])])
>> +
>> +PKG_CHECK_MODULES(SYSTEMD_LOGIN_207, [libsystemd-login >= 207],
>> +                  [have_systemd_login_207=yes], [have_systemd_login_207=no])
>> +AS_IF([test "x$have_systemd_login_207" = "xyes"],
>> +      [AC_DEFINE([HAVE_SYSTEMD_LOGIN_207], [1], [Have systemd-login >= 207])])
>> +
>> +PKG_CHECK_MODULES(LAUNCHER_UTIL, [libsystemd-login],,)
>> +PKG_CHECK_MODULES(WESTON_LAUNCH, [libdrm libsystemd-login],,)
>>
>>  if test x$enable_egl = xyes; then
>>    PKG_CHECK_MODULES(GLU, [glu], [have_glu=yes], [have_glu=no])
>> @@ -471,7 +468,6 @@ AC_MSG_RESULT([
>>       Build wcap utility              ${enable_wcap_tools}
>>       Build Tablet Shell              ${enable_tablet_shell}
>>
>> -     weston-launch utility           ${enable_weston_launch}
>>       weston-launch systemd support   ${have_systemd_login}
>>
>>       DRM Compositor                  ${enable_drm_compositor}
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index b0eae7c..2521b45 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1,5 +1,4 @@
>> -bin_PROGRAMS = weston                                \
>> -     $(weston_launch)
>> +bin_PROGRAMS = weston
>>
>>  AM_CPPFLAGS =                                        \
>>       -I$(top_srcdir)/shared                  \
>> @@ -63,22 +62,6 @@ endif
>>  DIST_SUBDIRS = xwayland
>>
>>
>> -if BUILD_WESTON_LAUNCH
>> -weston_launch = weston-launch
>> -weston_launch_SOURCES = weston-launch.c weston-launch.h
>> -weston_launch_CFLAGS= $(GCC_CFLAGS)
>> -weston_launch_CPPFLAGS = $(WESTON_LAUNCH_CFLAGS) $(SYSTEMD_LOGIN_CFLAGS) \
>> -              -DBINDIR='"$(bindir)"'
>> -weston_launch_LDADD = $(WESTON_LAUNCH_LIBS) $(SYSTEMD_LOGIN_LIBS)
>> -
>> -if ENABLE_SETUID_INSTALL
>> -install-exec-hook:
>> -     chown root $(DESTDIR)$(bindir)/weston-launch
>> -     chmod u+s $(DESTDIR)$(bindir)/weston-launch
>> -endif
>> -
>> -endif # BUILD_WESTON_LAUNCH
>> -
>>  pkgconfigdir = $(libdir)/pkgconfig
>>  pkgconfig_DATA = weston.pc
>>
>> @@ -103,6 +86,7 @@ module_LTLIBRARIES =                               \
>>       $(headless_backend)                     \
>>       $(fbdev_backend)                        \
>>       $(rdp_backend)
>> +module_PROGRAMS = weston-launch
>
> What about libexec_PROGRAMS?

I copied my code from systemd and they place executables there, too.
No idea which one is better..

>>  noinst_LTLIBRARIES =
>>
>> @@ -127,6 +111,17 @@ gl_renderer_la_SOURCES =                 \
>>       vertex-clipping.h
>>  endif
>>
>> +weston_launch_SOURCES = weston-launch.c weston-launch.h
>> +weston_launch_CFLAGS= $(GCC_CFLAGS)
>> +weston_launch_CPPFLAGS = $(WESTON_LAUNCH_CFLAGS)
>> +weston_launch_LDADD = $(WESTON_LAUNCH_LIBS)
>> +
>> +if ENABLE_SETUID_INSTALL
>> +install-data-hook:
>> +     chown root $(DESTDIR)$(moduledir)/weston-launch
>> +     chmod u+s $(DESTDIR)$(moduledir)/weston-launch
>> +endif
>> +
>>  if ENABLE_X11_COMPOSITOR
>>  x11_backend = x11-backend.la
>>  x11_backend_la_LDFLAGS = -module -avoid-version
>> @@ -146,11 +141,12 @@ if ENABLE_DRM_COMPOSITOR
>>  drm_backend = drm-backend.la
>>  drm_backend_la_LDFLAGS = -module -avoid-version
>>  drm_backend_la_LIBADD = $(COMPOSITOR_LIBS) $(DRM_COMPOSITOR_LIBS) \
>> -     ../shared/libshared.la -lrt
>> +     $(LAUNCHER_UTIL_LIBS) ../shared/libshared.la -lrt
>>  drm_backend_la_CFLAGS =                              \
>>       $(COMPOSITOR_CFLAGS)                    \
>>       $(EGL_CFLAGS)                           \
>>       $(DRM_COMPOSITOR_CFLAGS)                \
>> +     $(LAUNCHER_UTIL_CFLAGS)                 \
>>       $(GCC_CFLAGS)
>>  drm_backend_la_SOURCES =                     \
>>       compositor-drm.c                        \
>> @@ -192,10 +188,12 @@ rpi_backend_la_LDFLAGS = -module -avoid-version
>>  rpi_backend_la_LIBADD = $(COMPOSITOR_LIBS)   \
>>       $(RPI_COMPOSITOR_LIBS)                  \
>>       $(RPI_BCM_HOST_LIBS)                    \
>> +     $(LAUNCHER_UTIL_LIBS)                   \
>>       ../shared/libshared.la
>>  rpi_backend_la_CFLAGS =                              \
>>       $(GCC_CFLAGS)                           \
>>       $(COMPOSITOR_CFLAGS)                    \
>> +     $(LAUNCHER_UTIL_CFLAGS)                 \
>>       $(RPI_COMPOSITOR_CFLAGS)                \
>>       $(RPI_BCM_HOST_CFLAGS)
>>  rpi_backend_la_SOURCES =                     \
>> @@ -227,12 +225,14 @@ fbdev_backend_la_LDFLAGS = -module -avoid-version
>>  fbdev_backend_la_LIBADD = \
>>       $(COMPOSITOR_LIBS) \
>>       $(FBDEV_COMPOSITOR_LIBS) \
>> +     $(LAUNCHER_UTIL_LIBS) \
>>       ../shared/libshared.la
>>  fbdev_backend_la_CFLAGS = \
>>       $(COMPOSITOR_CFLAGS) \
>>       $(EGL_CFLAGS) \
>>       $(FBDEV_COMPOSITOR_CFLAGS) \
>>       $(PIXMAN_CFLAGS) \
>> +     $(LAUNCHER_UTIL_CFLAGS) \
>>       $(GCC_CFLAGS)
>>  fbdev_backend_la_SOURCES = \
>>       compositor-fbdev.c \
>> diff --git a/src/launcher-util.c b/src/launcher-util.c
>> index 4f77d11..3f19083 100644
>> --- a/src/launcher-util.c
>> +++ b/src/launcher-util.c
>> @@ -28,6 +28,7 @@
>>  #include <string.h>
>>
>>  #include <errno.h>
>> +#include <pwd.h>
>>  #include <signal.h>
>>  #include <sys/socket.h>
>>  #include <sys/types.h>
>> @@ -40,61 +41,44 @@
>>  #include <linux/kd.h>
>>  #include <linux/major.h>
>>
>> -#ifdef BUILD_DRM_COMPOSITOR
>> -#include <xf86drm.h>
>> -#endif
>> -
>>  #include "compositor.h"
>>  #include "launcher-util.h"
>>  #include "weston-launch.h"
>>
>> -#define DRM_MAJOR 226
>> -
>>  #ifndef KDSKBMUTE
>>  #define KDSKBMUTE    0x4B51
>>  #endif
>>
>> +#ifdef HAVE_SYSTEMD_LOGIN
>> +#include <systemd/sd-login.h>
>> +#endif
>> +
>>  union cmsg_data { unsigned char b[4]; int fd; };
>>
>>  struct weston_launcher {
>>       struct weston_compositor *compositor;
>> -     int fd;
>> +     struct passwd *pw;
>> +     struct weston_process launcher;
>> +     int fd, event_fd;
>>       struct wl_event_source *source;
>>
>> -     int kb_mode, tty, drm_fd;
>> -     struct wl_event_source *vt_source;
>> +     int kb_mode, tty;
>>  };
>>
>> -#ifdef BUILD_DRM_COMPOSITOR
>> -static int
>> -drm_drop_master(int drm_fd)
>> -{
>> -     return drmDropMaster(drm_fd);
>> -}
>> -static int
>> -drm_set_master(int drm_fd)
>> +static void weston_launcher_error(struct weston_launcher *launcher)
>>  {
>> -     return drmSetMaster(drm_fd);
>> +     /* Normally weston-launch will reset the tty, but
>> +      * in this case it died or something, so do it here so
>> +      * we don't end up with a stuck vt. */
>> +     weston_launcher_restore(launcher);
>> +     exit(-1);
>>  }
>> -static int
>> -drm_is_master(int drm_fd)
>> -{
>> -     drm_magic_t magic;
>> -
>> -     return drmGetMagic(drm_fd, &magic) == 0 &&
>> -             drmAuthMagic(drm_fd, magic) == 0;
>> -}
>> -#else
>> -static int drm_drop_master(int drm_fd) {return 0;}
>> -static int drm_set_master(int drm_fd) {return 0;}
>> -static int drm_is_master(int drm_fd) {return 1;}
>> -#endif
>>
>>  int
>>  weston_launcher_open(struct weston_launcher *launcher,
>>                    const char *path, int flags)
>>  {
>> -     int n, fd, ret = -1;
>> +     int n, ret = -1;
>>       struct msghdr msg;
>>       struct cmsghdr *cmsg;
>>       struct iovec iov;
>> @@ -102,29 +86,6 @@ weston_launcher_open(struct weston_launcher *launcher,
>>       char control[CMSG_SPACE(sizeof data->fd)];
>>       ssize_t len;
>>       struct weston_launcher_open *message;
>> -     struct stat s;
>> -
>> -     if (launcher->fd == -1) {
>> -             fd = open(path, flags | O_CLOEXEC);
>> -             if (fd == -1)
>> -                     return -1;
>> -
>> -             if (fstat(fd, &s) == -1) {
>> -                     close(fd);
>> -                     return -1;
>> -             }
>> -
>> -             if (major(s.st_rdev) == DRM_MAJOR) {
>> -                     launcher->drm_fd = fd;
>> -                     if (!drm_is_master(fd)) {
>> -                             weston_log("drm fd not master\n");
>> -                             close(fd);
>> -                             return -1;
>> -                     }
>> -             }
>> -
>> -             return fd;
>> -     }
>>
>>       n = sizeof(*message) + strlen(path) + 1;
>>       message = malloc(n);
>> @@ -153,8 +114,11 @@ weston_launcher_open(struct weston_launcher *launcher,
>>       } while (len < 0 && errno == EINTR);
>>
>>       if (len != sizeof ret ||
>> -         ret < 0)
>> +         ret < 0) {
>> +             fprintf(stderr, "invalid message received (%d, %d, %d) %m\n",
>> +                     (int)len, ret, errno);
>>               return -1;
>> +     }
>>
>>       cmsg = CMSG_FIRSTHDR(&msg);
>>       if (!cmsg ||
>> @@ -166,7 +130,7 @@ weston_launcher_open(struct weston_launcher *launcher,
>>
>>       data = (union cmsg_data *) CMSG_DATA(cmsg);
>>       if (data->fd == -1) {
>> -             fprintf(stderr, "missing drm fd in socket request\n");
>> +             fprintf(stderr, "missing fd in socket request\n");
>>               return -1;
>>       }
>>
>> @@ -178,35 +142,28 @@ weston_launcher_restore(struct weston_launcher *launcher)
>>  {
>>       struct vt_mode mode = { 0 };
>>
>> -     if (ioctl(launcher->tty, KDSKBMUTE, 0) &&
>> -         ioctl(launcher->tty, KDSKBMODE, launcher->kb_mode))
>> -             weston_log("failed to restore kb mode: %m\n");
>> -
>> -     if (ioctl(launcher->tty, KDSETMODE, KD_TEXT))
>> -             weston_log("failed to set KD_TEXT mode on tty: %m\n");
>> +     ioctl(launcher->tty, KDSKBMUTE, 0);
>> +     ioctl(launcher->tty, KDSKBMODE, launcher->kb_mode);
>> +     ioctl(launcher->tty, KDSETMODE, KD_TEXT);
>>
>>       mode.mode = VT_AUTO;
>> -     if (ioctl(launcher->tty, VT_SETMODE, &mode) < 0)
>> -             weston_log("could not reset vt handling\n");
>> +     ioctl(launcher->tty, VT_SETMODE, &mode);
>>  }
>>
>>  static int
>>  weston_launcher_data(int fd, uint32_t mask, void *data)
>>  {
>>       struct weston_launcher *launcher = data;
>> -     int len, ret;
>> +     int len, ret = -1;
>>
>>       if (mask & (WL_EVENT_HANGUP | WL_EVENT_ERROR)) {
>>               weston_log("launcher socket closed, exiting\n");
>> -             /* Normally the weston-launch will reset the tty, but
>> -              * in this case it died or something, so do it here so
>> -              * we don't end up with a stuck vt. */
>> -             weston_launcher_restore(launcher);
>> -             exit(-1);
>> +             weston_launcher_error(launcher);
>> +             return 1;
>>       }
>>
>>       do {
>> -             len = recv(launcher->fd, &ret, sizeof ret, 0);
>> +             len = recv(fd, &ret, sizeof ret, 0);
>>       } while (len < 0 && errno == EINTR);
>>
>>       switch (ret) {
>> @@ -228,115 +185,210 @@ weston_launcher_data(int fd, uint32_t mask, void *data)
>>       return 1;
>>  }
>>
>> -static int
>> -vt_handler(int signal_number, void *data)
>> +int
>> +weston_launcher_activate_vt(struct weston_launcher *launcher, int vt)
>>  {
>> -     struct weston_launcher *launcher = data;
>> -     struct weston_compositor *compositor = launcher->compositor;
>> -
>> -     if (compositor->session_active) {
>> -             compositor->session_active = 0;
>> -             wl_signal_emit(&compositor->session_signal, compositor);
>> -             drm_drop_master(launcher->drm_fd);
>> -             ioctl(launcher->tty, VT_RELDISP, 1);
>> -     } else {
>> -             ioctl(launcher->tty, VT_RELDISP, VT_ACKACQ);
>> -             drm_set_master(launcher->drm_fd);
>> -             compositor->session_active = 1;
>> -             wl_signal_emit(&compositor->session_signal, compositor);
>> -     }
>> +     ssize_t len;
>> +     struct weston_launcher_switchvt message;
>>
>> -     return 1;
>> +     /* VT_ACTIVATE is a privileged ioctl so ask weston-launch */
>> +
>> +     message.header.opcode = WESTON_LAUNCHER_SWITCHVT;
>> +     message.vt = vt;
>> +
>> +     do {
>> +             len = send(launcher->fd, &message, sizeof(message), 0);
>> +     } while (len < 0 && errno == EINTR);
>> +
>> +     return 0;
>>  }
>>
>> -static int
>> -setup_tty(struct weston_launcher *launcher, int tty)
>> +static void
>> +launcher_died(struct weston_process *proc, int status)
>>  {
>> -     struct wl_event_loop *loop;
>> -     struct vt_mode mode = { 0 };
>> -     struct stat buf;
>> -     char tty_device[32] ="<stdin>";
>> -     int ret, kd_mode;
>> -
>> -     if (tty == 0) {
>> -             launcher->tty = dup(tty);
>> -             if (launcher->tty == -1) {
>> -                     weston_log("couldn't dup stdin: %m\n");
>> -                     return -1;
>> -             }
>> -     } else {
>> -             snprintf(tty_device, sizeof tty_device, "/dev/tty%d", tty);
>> -             launcher->tty = open(tty_device, O_RDWR | O_CLOEXEC);
>> -             if (launcher->tty == -1) {
>> -                     weston_log("couldn't open tty %s: %m\n", tty_device);
>> -                     return -1;
>> -             }
>> -     }
>> +     struct weston_launcher *wl = container_of(proc, struct weston_launcher,
>> +                                               launcher);
>> +     weston_log("weston-launch died with status %d\n", status);
>> +     weston_launcher_error(wl);
>> +}
>>
>> -     if (fstat(launcher->tty, &buf) == -1 ||
>> -         major(buf.st_rdev) != TTY_MAJOR || minor(buf.st_rdev) == 0) {
>> -             weston_log("%s not a vt\n", tty_device);
>> -             weston_log("if running weston from ssh, "
>> -                        "use --tty to specify a tty\n");
>> -             goto err_close;
>> -     }
>> +static void
>> +setenv_fd(const char *env, int fd)
>> +{
>> +     char buf[32] = { 0 };
>> +
>> +     snprintf(buf, sizeof buf, "%d", fd);
>> +     setenv(env, buf, 1);
>> +}
>>
>> -     ret = ioctl(launcher->tty, KDGETMODE, &kd_mode);
>> -     if (ret) {
>> -             weston_log("failed to get VT mode: %m\n");
>> +static int
>> +spawn_weston_launch(struct weston_launcher *wl)
>> +{
>> +     char *child_argv[64];
>> +     int socks[2], esocks[2];
>> +     int chld, i, len, r = 0;
>> +
>> +     if (socketpair(AF_LOCAL, SOCK_SEQPACKET, 0, socks) < 0) {
>> +             weston_log("socketpair failed: %m\n");
>>               return -1;
>>       }
>> -     if (kd_mode != KD_TEXT) {
>> -             weston_log("%s is already in graphics mode, "
>> -                        "is another display server running?\n", tty_device);
>> -             goto err_close;
>> -     }
>>
>> -     ioctl(launcher->tty, VT_ACTIVATE, minor(buf.st_rdev));
>> -     ioctl(launcher->tty, VT_WAITACTIVE, minor(buf.st_rdev));
>> +     if (fcntl(socks[0], F_SETFD, FD_CLOEXEC) < 0) {
>> +             weston_log("setting cloexec failed: %m\n");
>> +             goto err_socks;
>> +     }
>>
>> -     if (ioctl(launcher->tty, KDGKBMODE, &launcher->kb_mode)) {
>> -             weston_log("failed to read keyboard mode: %m\n");
>> -             goto err_close;
>> +     if (socketpair(AF_LOCAL, SOCK_SEQPACKET, 0, esocks) < 0) {
>> +             weston_log("socketpair failed: %m\n");
>> +             goto err_socks;
>>       }
>>
>> -     if (ioctl(launcher->tty, KDSKBMUTE, 1) &&
>> -         ioctl(launcher->tty, KDSKBMODE, K_OFF)) {
>> -             weston_log("failed to set K_OFF keyboard mode: %m\n");
>> -             goto err_close;
>> +     if (fcntl(esocks[0], F_SETFD, FD_CLOEXEC) < 0) {
>> +             weston_log("setting cloexec failed: %m\n");
>> +             goto err_esocks;
>>       }
>>
>> -     ret = ioctl(launcher->tty, KDSETMODE, KD_GRAPHICS);
>> -     if (ret) {
>> -             weston_log("failed to set KD_GRAPHICS mode on tty: %m\n");
>> -             goto err_close;
>> +     chld = fork();
>> +     if (chld == -1) {
>> +             weston_log("cannot fork weston-launch: %m\n");
>> +             goto err_esocks;
>>       }
>>
>> -     mode.mode = VT_PROCESS;
>> -     mode.relsig = SIGUSR1;
>> -     mode.acqsig = SIGUSR1;
>> -     if (ioctl(launcher->tty, VT_SETMODE, &mode) < 0) {
>> -             weston_log("failed to take control of vt handling\n");
>> -             goto err_close;
>> +     if (chld == 0) {
>> +             setenv_fd("WESTON_LAUNCHER_SOCK", socks[1]);
>> +             setenv_fd("WESTON_LAUNCHER_EVENT_SOCK", esocks[1]);
>> +
>> +             fcntl(wl->tty, F_SETFD, fcntl(wl->tty, F_GETFD) & ~FD_CLOEXEC);
>> +             setenv_fd("WESTON_LAUNCHER_TTY", wl->tty);
>> +
>> +             i = 0;
>> +             child_argv[i++] = wl->pw->pw_shell;
>> +             child_argv[i++] = "-l";
>> +             child_argv[i++] = "-c";
>> +             child_argv[i++] = MODULEDIR "/weston-launch \"$@\"";
>> +             child_argv[i++] = "weston-launch";
>> +             child_argv[i++] = "-v";
>> +             child_argv[i] = NULL;
>
> We don't need to run weston-launch through pw_shell do we?

Yepp, execve() is simpler, fixed.

>> +             execv(child_argv[0], child_argv);
>> +             _exit(1);
>>       }
>>
>> -     loop = wl_display_get_event_loop(launcher->compositor->wl_display);
>> -     launcher->vt_source =
>> -             wl_event_loop_add_signal(loop, SIGUSR1, vt_handler, launcher);
>> -     if (!launcher->vt_source)
>> -             goto err_close;
>> +     close(socks[1]);
>> +     socks[1] = -1;
>> +     close(esocks[1]);
>> +     esocks[1] = -1;
>> +
>> +     wl->launcher.pid = chld;
>> +     wl->launcher.cleanup = launcher_died;
>> +     wl->fd = socks[0];
>> +     wl->event_fd = esocks[0];
>> +
>> +     weston_log("wait for weston-launch startup..\n");
>> +     do {
>> +             len = recv(wl->event_fd, &r, sizeof r, 0);
>> +     } while (len < 0 && errno == EINTR);
>> +
>> +     if (len >= 0)
>> +             errno = (len == sizeof r) ? r : EINVAL;
>> +
>> +     if (len != sizeof(r) || r) {
>> +             weston_log("cannot start weston-launch (%d, %d, %d): %m\n",
>> +                        (int)len, r, errno);
>> +             goto err_esocks;
>> +     }
>>
>>       return 0;
>>
>> - err_close:
>> -     close(launcher->tty);
>> +err_esocks:
>> +     close(esocks[0]);
>> +     if (esocks[1] >= 0)
>> +             close(esocks[1]);
>> +err_socks:
>> +     close(socks[0]);
>> +     if (socks[1] >= 0)
>> +             close(socks[1]);
>>       return -1;
>>  }
>>
>> -int
>> -weston_launcher_activate_vt(struct weston_launcher *launcher, int vt)
>> +/*
>> + * For sessions on seat0 we need to open our controlling VT to set
>> + * graphics-mode and disable kb-input. For legacy mode we also need it to
>> + * get notified about VT switches.
>> + *
>> + * We choose the VT in this order:
>> + *  (1) If the WESTON_TTY_FD environment variable is set, we use it
>> + *  (2) If --tty=$id is passed, we use /dev/tty$id
>> + *  (2) If systemd support is available, the session's VT is used
>> + *  (3) If STDIN is a TTY, we use it
>> + *  (4) We use /dev/tty
>> + */
>> +static int
>> +setup_tty(struct weston_launcher *wl, int ttyid)
>>  {
>> -     return ioctl(launcher->tty, VT_ACTIVATE, vt);
>> +     struct stat st;
>> +     char *t, buf[32], *s = NULL;
>> +     int fd = -1, r;
>> +     unsigned int num;
>> +
>> +     t = "$WESTON_TTY_FD";
>
> What's this for?

Ups, we don't need this. Removed.

>> +     fd = weston_environment_get_fd("WESTON_TTY_FD");
>> +     if (fd < 0) {
>> +             if (ttyid > 0) {
>> +                     snprintf(buf, sizeof(buf), "/dev/tty%d", ttyid);
>> +                     buf[sizeof(buf) - 1] = 0;
>> +                     t = buf;
>> +#ifdef HAVE_SYSTEMD_LOGIN_207
>> +             } else if (sd_pid_get_session(getpid(), &s) >= 0 &&
>> +                        sd_session_get_vt(s, &num) >= 0) {
>> +                     snprintf(buf, sizeof(buf), "/dev/tty%u", num);
>> +                     buf[sizeof(buf) - 1] = 0;
>> +                     t = buf;
>> +#endif
>> +             } else if (ttyname(STDIN_FILENO)) {
>> +                     t = ctermid(NULL);
>> +                     fd = dup(STDIN_FILENO);
>> +             } else {
>> +                     t = ctermid(NULL);
>> +             }
>> +
>> +             if (fd < 0)
>> +                     fd = open(t, O_RDWR|O_CLOEXEC|O_NONBLOCK);
>> +
>> +             if (fd < 0) {
>> +                     weston_log("cannot open TTY %s: %m\n", t);
>> +                     return -1;
>> +             }
>> +     }
>> +
>> +     free(s);
>> +
>> +     if (fstat(fd, &st) == -1 ||
>> +         major(st.st_rdev) != TTY_MAJOR || minor(st.st_rdev) <= 0 ||
>> +         minor(st.st_rdev) >= 64) {
>> +             weston_log("TTY %s is no virtual terminal\n", t);
>> +             close(fd);
>> +             return -1;
>> +     }
>> +     weston_log("using TTY %s\n", t);
>> +
>> +     r = setsid();
>> +     if (r < 0 && errno != EPERM)
>> +             weston_log("setsid() failed: %m\n");
>> +
>> +     r = ioctl(fd, TIOCSCTTY, 0);
>> +     if (r < 0)
>> +             weston_log("VT %s already in use\n", t);
>> +
>> +     if (ioctl(fd, KDGKBMODE, &wl->kb_mode)) {
>> +             weston_log("cannot get current keyboard mode: %m\n");
>> +             close(fd);
>> +             return -1;
>> +     }
>> +
>> +     wl->tty = fd;
>> +
>> +     return 0;
>>  }
>>
>>  struct weston_launcher *
>> @@ -344,49 +396,57 @@ weston_launcher_connect(struct weston_compositor *compositor, int tty)
>>  {
>>       struct weston_launcher *launcher;
>>       struct wl_event_loop *loop;
>> +     int r;
>>
>>       launcher = malloc(sizeof *launcher);
>>       if (launcher == NULL)
>>               return NULL;
>>
>>       launcher->compositor = compositor;
>> -     launcher->drm_fd = -1;
>> -     launcher->fd = weston_environment_get_fd("WESTON_LAUNCHER_SOCK");
>> -     if (launcher->fd != -1) {
>> -             launcher->tty = weston_environment_get_fd("WESTON_TTY_FD");
>> -             loop = wl_display_get_event_loop(compositor->wl_display);
>> -             launcher->source = wl_event_loop_add_fd(loop, launcher->fd,
>> -                                                     WL_EVENT_READABLE,
>> -                                                     weston_launcher_data,
>> -                                                     launcher);
>> -             if (launcher->source == NULL) {
>> -                     free(launcher);
>> -                     return NULL;
>> -             }
>> -     } else if (geteuid() == 0) {
>> -             if (setup_tty(launcher, tty) == -1) {
>> -                     free(launcher);
>> -                     return NULL;
>> -             }
>> -     } else {
>> -             free(launcher);
>> -             return NULL;
>> -     }
>> +     launcher->pw = getpwuid(getuid());
>> +     launcher->tty = -1;
>> +     launcher->fd = -1;
>> +     launcher->event_fd = -1;
>> +
>> +     r = setup_tty(launcher, tty);
>> +     if (r < 0)
>> +             goto err_free;
>> +
>> +     r = spawn_weston_launch(launcher);
>> +     if (r < 0)
>> +             goto err_tty;
>> +
>> +     loop = wl_display_get_event_loop(compositor->wl_display);
>> +     launcher->source = wl_event_loop_add_fd(loop, launcher->event_fd,
>> +                                             WL_EVENT_READABLE,
>> +                                             weston_launcher_data,
>> +                                             launcher);
>> +     if (launcher->source == NULL)
>> +             goto err_launch;
>> +
>> +     weston_watch_process(&launcher->launcher);
>>
>>       return launcher;
>> +
>> +err_launch:
>> +     close(launcher->event_fd);
>> +     close(launcher->fd);
>> +err_tty:
>> +     weston_launcher_restore(launcher);
>> +     close(launcher->tty);
>> +err_free:
>> +     free(launcher);
>> +     return NULL;
>>  }
>>
>>  void
>>  weston_launcher_destroy(struct weston_launcher *launcher)
>>  {
>> -     if (launcher->fd != -1) {
>> -             close(launcher->fd);
>> -             wl_event_source_remove(launcher->source);
>> -     } else {
>> -             weston_launcher_restore(launcher);
>> -             wl_event_source_remove(launcher->vt_source);
>> -     }
>> -
>> +     wl_list_remove(&launcher->launcher.link);
>> +     wl_event_source_remove(launcher->source);
>> +     close(launcher->event_fd);
>> +     close(launcher->fd);
>> +     weston_launcher_restore(launcher);
>>       close(launcher->tty);
>>       free(launcher);
>>  }
>> diff --git a/src/weston-launch.c b/src/weston-launch.c
>> index e5301c7..c6a8d34 100644
>> --- a/src/weston-launch.c
>> +++ b/src/weston-launch.c
>> @@ -48,9 +48,10 @@
>>
>>  #include <pwd.h>
>>  #include <grp.h>
>> -#include <security/pam_appl.h>
>>
>> +#ifdef BUILD_DRM_COMPOSITOR
>>  #include <xf86drm.h>
>> +#endif
>>
>>  #ifdef HAVE_SYSTEMD_LOGIN
>>  #include <systemd/sd-login.h>
>> @@ -64,23 +65,36 @@
>>  #define KDSKBMUTE    0x4B51
>>  #endif
>>
>> -#define MAX_ARGV_SIZE 256
>> +#ifdef BUILD_DRM_COMPOSITOR
>> +static int
>> +drm_drop_master(int drm_fd)
>> +{
>> +     if (drm_fd != -1)
>> +             return drmDropMaster(drm_fd);
>> +     return -EBADF;
>> +}
>> +static int
>> +drm_set_master(int drm_fd)
>> +{
>> +     if (drm_fd != -1)
>> +             return drmSetMaster(drm_fd);
>> +     return -EBADF;
>> +}
>> +#else
>> +static int drm_drop_master(int drm_fd) {return 0;}
>> +static int drm_set_master(int drm_fd) {return 0;}
>> +#endif
>>
>>  struct weston_launch {
>> -     struct pam_conv pc;
>> -     pam_handle_t *ph;
>>       int tty;
>> -     int ttynr;
>> -     int sock[2];
>> +     int sock;
>> +     int event_sock;
>>       int drm_fd;
>>       int kb_mode;
>> -     struct passwd *pw;
>>
>>       int signalfd;
>>
>> -     pid_t child;
>>       int verbose;
>> -     char *new_user;
>>  };
>>
>>  union cmsg_data { unsigned char b[4]; int fd; };
>> @@ -154,79 +168,15 @@ weston_launch_allowed(struct weston_launch *wl)
>>       return 0;
>>  }
>>
>> -static int
>> -pam_conversation_fn(int msg_count,
>> -                 const struct pam_message **messages,
>> -                 struct pam_response **responses,
>> -                 void *user_data)
>> -{
>> -     return PAM_SUCCESS;
>> -}
>> -
>> -static int
>> -setup_pam(struct weston_launch *wl)
>> -{
>> -     int err;
>> -
>> -     wl->pc.conv = pam_conversation_fn;
>> -     wl->pc.appdata_ptr = wl;
>> -
>> -     err = pam_start("login", wl->pw->pw_name, &wl->pc, &wl->ph);
>> -     if (err != PAM_SUCCESS) {
>> -             fprintf(stderr, "failed to start pam transaction: %d: %s\n",
>> -                     err, pam_strerror(wl->ph, err));
>> -             return -1;
>> -     }
>> -
>> -     err = pam_set_item(wl->ph, PAM_TTY, ttyname(wl->tty));
>> -     if (err != PAM_SUCCESS) {
>> -             fprintf(stderr, "failed to set PAM_TTY item: %d: %s\n",
>> -                     err, pam_strerror(wl->ph, err));
>> -             return -1;
>> -     }
>> -
>> -     err = pam_open_session(wl->ph, 0);
>> -     if (err != PAM_SUCCESS) {
>> -             fprintf(stderr, "failed to open pam session: %d: %s\n",
>> -                     err, pam_strerror(wl->ph, err));
>> -             return -1;
>> -     }
>> -
>> -     return 0;
>> -}
>> -
>> -static int
>> -setup_launcher_socket(struct weston_launch *wl)
>> -{
>> -     if (socketpair(AF_LOCAL, SOCK_SEQPACKET, 0, wl->sock) < 0)
>> -             error(1, errno, "socketpair failed");
>> -
>> -     if (fcntl(wl->sock[0], F_SETFD, FD_CLOEXEC) < 0)
>> -             error(1, errno, "fcntl failed");
>> -
>> -     return 0;
>> -}
>> -
>> -static int
>> +static void
>>  setup_signals(struct weston_launch *wl)
>>  {
>>       int ret;
>>       sigset_t mask;
>> -     struct sigaction sa;
>> -
>> -     memset(&sa, 0, sizeof sa);
>> -     sa.sa_handler = SIG_DFL;
>> -     sa.sa_flags = SA_NOCLDSTOP | SA_RESTART;
>> -     ret = sigaction(SIGCHLD, &sa, NULL);
>> -     assert(ret == 0);
>> -
>> -     sa.sa_handler = SIG_IGN;
>> -     sa.sa_flags = 0;
>> -     sigaction(SIGHUP, &sa, NULL);
>>
>>       ret = sigemptyset(&mask);
>>       assert(ret == 0);
>> -     sigaddset(&mask, SIGCHLD);
>> +     sigaddset(&mask, SIGHUP);
>>       sigaddset(&mask, SIGINT);
>>       sigaddset(&mask, SIGTERM);
>>       sigaddset(&mask, SIGUSR1);
>> @@ -236,18 +186,7 @@ setup_signals(struct weston_launch *wl)
>>
>>       wl->signalfd = signalfd(-1, &mask, SFD_NONBLOCK | SFD_CLOEXEC);
>>       if (wl->signalfd < 0)
>> -             return -errno;
>> -
>> -     return 0;
>> -}
>> -
>> -static void
>> -setenv_fd(const char *env, int fd)
>> -{
>> -     char buf[32];
>> -
>> -     snprintf(buf, sizeof buf, "%d", fd);
>> -     setenv(env, buf, 1);
>> +             error(1, errno, "cannot create signalfd");
>>  }
>>
>>  static int
>> @@ -256,7 +195,7 @@ send_reply(struct weston_launch *wl, int reply)
>>       int len;
>>
>>       do {
>> -             len = send(wl->sock[0], &reply, sizeof reply, 0);
>> +             len = send(wl->event_sock, &reply, sizeof reply, 0);
>>       } while (len < 0 && errno == EINTR);
>>
>>       return len;
>> @@ -327,7 +266,7 @@ err0:
>>               fprintf(stderr, "weston-launch: opened %s: ret: %d, fd: %d\n",
>>                       message->path, ret, fd);
>>       do {
>> -             len = sendmsg(wl->sock[0], &nmsg, 0);
>> +             len = sendmsg(wl->sock, &nmsg, 0);
>>       } while (len < 0 && errno == EINTR);
>>
>>       if (len < 0)
>> @@ -340,6 +279,24 @@ err0:
>>  }
>>
>>  static int
>> +handle_switchvt(struct weston_launch *wl, struct msghdr *msg, ssize_t len)
>> +{
>> +     struct weston_launcher_switchvt *message;
>> +
>> +     message = msg->msg_iov->iov_base;
>> +     if ((size_t)len < sizeof(*message) || message->vt <= 0)
>> +             return -EINVAL;
>> +
>> +     ioctl(wl->tty, VT_ACTIVATE, message->vt);
>> +
>> +     if (wl->verbose)
>> +             fprintf(stderr, "weston-launch: switch VT to %d\n",
>> +                     message->vt);
>> +
>> +     return 0;
>> +}
>> +
>> +static int
>>  handle_socket_msg(struct weston_launch *wl)
>>  {
>>       char control[CMSG_SPACE(sizeof(int))];
>> @@ -359,7 +316,7 @@ handle_socket_msg(struct weston_launch *wl)
>>       msg.msg_controllen = sizeof control;
>>
>>       do {
>> -             len = recvmsg(wl->sock[0], &msg, 0);
>> +             len = recvmsg(wl->sock, &msg, 0);
>>       } while (len < 0 && errno == EINTR);
>>
>>       if (len < 1)
>> @@ -370,6 +327,9 @@ handle_socket_msg(struct weston_launch *wl)
>>       case WESTON_LAUNCHER_OPEN:
>>               ret = handle_open(wl, &msg, len);
>>               break;
>> +     case WESTON_LAUNCHER_SWITCHVT:
>> +             ret = handle_switchvt(wl, &msg, len);
>> +             break;
>>       }
>>
>>       return ret;
>> @@ -379,18 +339,9 @@ static void
>>  quit(struct weston_launch *wl, int status)
>>  {
>>       struct vt_mode mode = { 0 };
>> -     int err;
>>
>>       close(wl->signalfd);
>> -     close(wl->sock[0]);
>> -
>> -     if (wl->new_user) {
>> -             err = pam_close_session(wl->ph, 0);
>> -             if (err)
>> -                     fprintf(stderr, "pam_close_session failed: %d: %s\n",
>> -                             err, pam_strerror(wl->ph, err));
>> -             pam_end(wl->ph, err);
>> -     }
>> +     close(wl->sock);
>>
>>       if (ioctl(wl->tty, KDSKBMUTE, 0) &&
>>           ioctl(wl->tty, KDSKBMODE, wl->kb_mode))
>> @@ -410,7 +361,6 @@ static int
>>  handle_signal(struct weston_launch *wl)
>>  {
>>       struct signalfd_siginfo sig;
>> -     int pid, status, ret;
>>
>>       if (read(wl->signalfd, &sig, sizeof sig) != sizeof sig) {
>>               error(0, errno, "reading signalfd failed");
>> @@ -418,38 +368,19 @@ handle_signal(struct weston_launch *wl)
>>       }
>>
>>       switch (sig.ssi_signo) {
>> -     case SIGCHLD:
>> -             pid = waitpid(-1, &status, 0);
>> -             if (pid == wl->child) {
>> -                     wl->child = 0;
>> -                     if (WIFEXITED(status))
>> -                             ret = WEXITSTATUS(status);
>> -                     else if (WIFSIGNALED(status))
>> -                             /*
>> -                              * If weston dies because of signal N, we
>> -                              * return 10+N. This is distinct from
>> -                              * weston-launch dying because of a signal
>> -                              * (128+N).
>> -                              */
>> -                             ret = 10 + WTERMSIG(status);
>> -                     else
>> -                             ret = 0;
>> -                     quit(wl, ret);
>> -             }
>> -             break;
>> +     case SIGHUP:
>>       case SIGTERM:
>>       case SIGINT:
>> -             if (wl->child)
>> -                     kill(wl->child, sig.ssi_signo);
>> +             quit(wl, 1);
>>               break;
>>       case SIGUSR1:
>>               send_reply(wl, WESTON_LAUNCHER_DEACTIVATE);
>> -             drmDropMaster(wl->drm_fd);
>> +             drm_drop_master(wl->drm_fd);
>>               ioctl(wl->tty, VT_RELDISP, 1);
>>               break;
>>       case SIGUSR2:
>>               ioctl(wl->tty, VT_RELDISP, VT_ACKACQ);
>> -             drmSetMaster(wl->drm_fd);
>> +             drm_set_master(wl->drm_fd);
>>               send_reply(wl, WESTON_LAUNCHER_ACTIVATE);
>>               break;
>>       default:
>> @@ -459,162 +390,66 @@ handle_signal(struct weston_launch *wl)
>>       return 0;
>>  }
>>
>> -static int
>> -setup_tty(struct weston_launch *wl, const char *tty)
>> +static void
>> +setup_tty(struct weston_launch *wl)
>>  {
>> -     struct stat buf;
>>       struct vt_mode mode = { 0 };
>> -     char *t;
>> -
>> -     if (!wl->new_user) {
>> -             wl->tty = STDIN_FILENO;
>> -     } else if (tty) {
>> -             t = ttyname(STDIN_FILENO);
>> -             if (t && strcmp(t, tty) == 0)
>> -                     wl->tty = STDIN_FILENO;
>> -             else
>> -                     wl->tty = open(tty, O_RDWR | O_NOCTTY);
>> -     } else {
>> -             int tty0 = open("/dev/tty0", O_WRONLY | O_CLOEXEC);
>> -             char filename[16];
>> -
>> -             if (tty0 < 0)
>> -                     error(1, errno, "could not open tty0");
>> -
>> -             if (ioctl(tty0, VT_OPENQRY, &wl->ttynr) < 0 || wl->ttynr == -1)
>> -                     error(1, errno, "failed to find non-opened console");
>> -
>> -             snprintf(filename, sizeof filename, "/dev/tty%d", wl->ttynr);
>> -             wl->tty = open(filename, O_RDWR | O_NOCTTY);
>> -             close(tty0);
>> -     }
>> -
>> -     if (wl->tty < 0)
>> -             error(1, errno, "failed to open tty");
>> -
>> -     if (fstat(wl->tty, &buf) == -1 ||
>> -         major(buf.st_rdev) != TTY_MAJOR || minor(buf.st_rdev) == 0)
>> -             error(1, 0, "weston-launch must be run from a virtual terminal");
>> -
>> -     if (tty) {
>> -             if (fstat(wl->tty, &buf) < 0)
>> -                     error(1, errno, "stat %s failed", tty);
>> -
>> -             if (major(buf.st_rdev) != TTY_MAJOR)
>> -                     error(1, 0, "invalid tty device: %s", tty);
>> -
>> -             wl->ttynr = minor(buf.st_rdev);
>> -     }
>> +     struct stat st;
>>
>>       if (ioctl(wl->tty, KDGKBMODE, &wl->kb_mode))
>> -             error(1, errno, "failed to get current keyboard mode: %m\n");
>> +             error(1, errno, "failed to get current keyboard mode");
>> +     if (wl->kb_mode == K_OFF)
>> +             wl->kb_mode = K_UNICODE;
>>
>>       if (ioctl(wl->tty, KDSKBMUTE, 1) &&
>>           ioctl(wl->tty, KDSKBMODE, K_OFF))
>> -             error(1, errno, "failed to set K_OFF keyboard mode: %m\n");
>> +             error(1, errno, "failed to set K_OFF keyboard mode");
>>
>>       if (ioctl(wl->tty, KDSETMODE, KD_GRAPHICS))
>> -             error(1, errno, "failed to set KD_GRAPHICS mode on tty: %m\n");
>> +             error(1, errno, "failed to set KD_GRAPHICS mode on tty");
>>
>>       mode.mode = VT_PROCESS;
>>       mode.relsig = SIGUSR1;
>>       mode.acqsig = SIGUSR2;
>>       if (ioctl(wl->tty, VT_SETMODE, &mode) < 0)
>> -             error(1, errno, "failed to take control of vt handling\n");
>> -
>> -     return 0;
>> -}
>> -
>> -static void
>> -setup_session(struct weston_launch *wl)
>> -{
>> -     char **env;
>> -     char *term;
>> -     int i;
>> -
>> -     if (wl->tty != STDIN_FILENO) {
>> -             if (setsid() < 0)
>> -                     error(1, errno, "setsid failed");
>> -             if (ioctl(wl->tty, TIOCSCTTY, 0) < 0)
>> -                     error(1, errno, "TIOCSCTTY failed - tty is in use");
>> -     }
>> +             error(1, errno, "failed to take control of vt handling");
>>
>> -     term = getenv("TERM");
>> -     clearenv();
>> -     if (term)
>> -             setenv("TERM", term, 1);
>> -     setenv("USER", wl->pw->pw_name, 1);
>> -     setenv("LOGNAME", wl->pw->pw_name, 1);
>> -     setenv("HOME", wl->pw->pw_dir, 1);
>> -     setenv("SHELL", wl->pw->pw_shell, 1);
>> -
>> -     env = pam_getenvlist(wl->ph);
>> -     if (env) {
>> -             for (i = 0; env[i]; ++i) {
>> -                     if (putenv(env[i]) < 0)
>> -                             error(0, 0, "putenv %s failed", env[i]);
>> -             }
>> -             free(env);
>> -     }
>> -}
>> +     if (fstat(wl->tty, &st) < 0)
>> +             error(1, errno, "cannot get VT number: %m");
>>
>> -static void
>> -drop_privileges(struct weston_launch *wl)
>> -{
>> -     if (setgid(wl->pw->pw_gid) < 0 ||
>> -#ifdef HAVE_INITGROUPS
>> -         initgroups(wl->pw->pw_name, wl->pw->pw_gid) < 0 ||
>> -#endif
>> -         setuid(wl->pw->pw_uid) < 0)
>> -             error(1, errno, "dropping privileges failed");
>> +     fprintf(stderr, "activate VT %d\n", minor(st.st_rdev));
>> +     if (ioctl(wl->tty, VT_ACTIVATE, minor(st.st_rdev)) < 0 ||
>> +         ioctl(wl->tty, VT_WAITACTIVE, minor(st.st_rdev)) < 0)
>> +             error(1, errno, "cannot activate VT %d", minor(st.st_rdev));
>>  }
>>
>> -static void
>> -launch_compositor(struct weston_launch *wl, int argc, char *argv[])
>> +static int
>> +get_env_fd(const char *env)
>>  {
>> -     char *child_argv[MAX_ARGV_SIZE];
>> -     sigset_t mask;
>> -     int i;
>> +     char *e, *end;
>> +     int fd, flags;
>>
>> -     if (wl->verbose)
>> -             printf("weston-launch: spawned weston with pid: %d\n", getpid());
>> -     if (wl->new_user)
>> -             setup_session(wl);
>> -
>> -     if (geteuid() == 0)
>> -             drop_privileges(wl);
>> +     e = getenv(env);
>> +     if (!e)
>> +             return -1;
>> +     fd = strtol(e, &end, 0);
>> +     if (*end != '\0')
>> +             return -1;
>>
>> -     setenv_fd("WESTON_TTY_FD", wl->tty);
>> -     setenv_fd("WESTON_LAUNCHER_SOCK", wl->sock[1]);
>> +     flags = fcntl(fd, F_GETFD);
>> +     if (flags == -1)
>> +             return -1;
>>
>> -     unsetenv("DISPLAY");
>> +     fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>> +     unsetenv(env);
>>
>> -     /* Do not give our signal mask to the new process. */
>> -     sigemptyset(&mask);
>> -     sigaddset(&mask, SIGTERM);
>> -     sigaddset(&mask, SIGCHLD);
>> -     sigaddset(&mask, SIGINT);
>> -     sigprocmask(SIG_UNBLOCK, &mask, NULL);
>> -
>> -     child_argv[0] = wl->pw->pw_shell;
>> -     child_argv[1] = "-l";
>> -     child_argv[2] = "-c";
>> -     child_argv[3] = BINDIR "/weston \"$@\"";
>> -     child_argv[4] = "weston";
>> -     for (i = 0; i < argc; ++i)
>> -             child_argv[5 + i] = argv[i];
>> -     child_argv[5 + i] = NULL;
>> -
>> -     execv(child_argv[0], child_argv);
>> -     error(1, errno, "exec failed");
>> +     return fd;
>>  }
>>
>>  static void
>>  help(const char *name)
>>  {
>> -     fprintf(stderr, "Usage: %s [args...] [-- [weston args..]]\n", name);
>> -     fprintf(stderr, "  -u, --user      Start session as specified username\n");
>> -     fprintf(stderr, "  -t, --tty       Start session on alternative tty\n");
>> +     fprintf(stderr, "Usage: %s [args...]\n", name);
>>       fprintf(stderr, "  -v, --verbose   Be verbose\n");
>>       fprintf(stderr, "  -h, --help      Display this help message\n");
>>  }
>> @@ -624,27 +459,17 @@ main(int argc, char *argv[])
>>  {
>>       struct weston_launch wl;
>>       int i, c;
>> -     char *tty = NULL;
>>       struct option opts[] = {
>> -             { "user",    required_argument, NULL, 'u' },
>> -             { "tty",     required_argument, NULL, 't' },
>>               { "verbose", no_argument,       NULL, 'v' },
>>               { "help",    no_argument,       NULL, 'h' },
>>               { 0,         0,                 NULL,  0  }
>> -     };
>> +     };
>>
>>       memset(&wl, 0, sizeof wl);
>> +     wl.kb_mode = K_UNICODE;
>>
>> -     while ((c = getopt_long(argc, argv, "u:t::vh", opts, &i)) != -1) {
>> +     while ((c = getopt_long(argc, argv, "vh", opts, &i)) != -1) {
>>               switch (c) {
>> -             case 'u':
>> -                     wl.new_user = optarg;
>> -                     if (getuid() != 0)
>> -                             error(1, 0, "Permission denied. -u allowed for root only");
>> -                     break;
>> -             case 't':
>> -                     tty = optarg;
>> -                     break;
>>               case 'v':
>>                       wl.verbose = 1;
>>                       break;
>> @@ -654,18 +479,9 @@ main(int argc, char *argv[])
>>               }
>>       }
>>
>> -     if ((argc - optind) > (MAX_ARGV_SIZE - 6))
>> -             error(1, E2BIG, "Too many arguments to pass to weston");
>> -
>> -     if (wl.new_user)
>> -             wl.pw = getpwnam(wl.new_user);
>> -     else
>> -             wl.pw = getpwuid(getuid());
>> -     if (wl.pw == NULL)
>> -             error(1, errno, "failed to get username");
>> -
>>       if (!weston_launch_allowed(&wl))
>>               error(1, 0, "Permission denied. You should either:\n"
>> +                   " - run as root.\n"
>>  #ifdef HAVE_SYSTEMD_LOGIN
>>                     " - run from an active and local (systemd) session.\n"
>>  #else
>> @@ -673,36 +489,23 @@ main(int argc, char *argv[])
>>  #endif
>>                     " - or add yourself to the 'weston-launch' group.");
>>
>> -     if (setup_tty(&wl, tty) < 0)
>> -             exit(EXIT_FAILURE);
>> -
>> -     if (wl.new_user && setup_pam(&wl) < 0)
>> -             exit(EXIT_FAILURE);
>> -
>> -     if (setup_launcher_socket(&wl) < 0)
>> -             exit(EXIT_FAILURE);
>> -
>> -     if (setup_signals(&wl) < 0)
>> -             exit(EXIT_FAILURE);
>> -
>> -     wl.child = fork();
>> -     if (wl.child == -1) {
>> -             error(1, errno, "fork failed");
>> -             exit(EXIT_FAILURE);
>> -     }
>> +     wl.tty = get_env_fd("WESTON_LAUNCHER_TTY");
>> +     wl.sock = get_env_fd("WESTON_LAUNCHER_SOCK");
>> +     wl.event_sock = get_env_fd("WESTON_LAUNCHER_EVENT_SOCK");
>> +     if (wl.tty < 0 || wl.sock < 0 || wl.event_sock < 0)
>> +             error(1, 0, "cannot get TTY/SOCK from environment.");
>>
>> -     if (wl.child == 0)
>> -             launch_compositor(&wl, argc - optind, argv + optind);
>> +     setup_signals(&wl);
>> +     setup_tty(&wl);
>>
>> -     close(wl.sock[1]);
>> -     if (wl.tty != STDIN_FILENO)
>> -             close(wl.tty);
>> +     /* send startup notification */
>> +     send_reply(&wl, WESTON_LAUNCHER_SUCCESS);
>>
>>       while (1) {
>>               struct pollfd fds[2];
>>               int n;
>>
>> -             fds[0].fd = wl.sock[0];
>> +             fds[0].fd = wl.sock;
>>               fds[0].events = POLLIN;
>>               fds[1].fd = wl.signalfd;
>>               fds[1].events = POLLIN;
>> @@ -712,6 +515,8 @@ main(int argc, char *argv[])
>>                       error(0, errno, "poll failed");
>>               if (fds[0].revents & POLLIN)
>>                       handle_socket_msg(&wl);
>> +             if (fds[0].revents & (POLLHUP | POLLERR))
>> +                     quit(&wl, 0);
>>               if (fds[1].revents)
>>                       handle_signal(&wl);
>>       }
>> diff --git a/src/weston-launch.h b/src/weston-launch.h
>> index e20c4c7..85d2fc8 100644
>> --- a/src/weston-launch.h
>> +++ b/src/weston-launch.h
>> @@ -25,6 +25,7 @@
>>
>>  enum weston_launcher_opcode {
>>       WESTON_LAUNCHER_OPEN,
>> +     WESTON_LAUNCHER_SWITCHVT,
>>  };
>>
>>  enum weston_launcher_event {
>> @@ -43,4 +44,9 @@ struct weston_launcher_open {
>>       char path[0];
>>  };
>>
>> +struct weston_launcher_switchvt {
>> +     struct weston_launcher_message header;
>> +     int vt;
>> +};
>> +
>>  #endif
>> --
>> 1.8.4
>>

Thanks
David


More information about the wayland-devel mailing list