Merge lp:~mterry/lightdm/session-issues into lp:lightdm

[Michael Terry]

There are two in-session bugs fixed here.

One is a lightdm crash when an in-session greeter tries to authenticate any user a second time.

The second fix stops lightdm from closing a session if a greeter tries to start a new authentication after already succeeding in authenticating a user.

== The crash ==

Fixed by r2378 in this branch. It seemed the seat was shutting down the greeter-session's authentication session. But the greeter wasn't watching its auth session to see when it got closed. And it would try to use it later.

So I've simply added a watch for that signal.

== Closing a session ==

Fixed by r2379 in this branch. If a greeter-session's authentication session becomes a real boy, we don't want to stop that session when doing a new authentication.

So if the auth session is running a command, I've changed the code to not close the session when resetting the greeter state.

About the closing-session bug, I think it would have been a problem with old greeters too, not just in-session ones? But they typically don't start a new auth, and instead get shut down.

I've added a test for that closing-session bug, but I did embed it inside a semi-unrelated test that used a mir greeter session and switched users and all. I didn't try to find a smallest-possible test for it.

Both of these may not be the perfect solution? I just tried to fix what I saw. Maybe you think these issues need more code rethinking?

[lp:~mterry/lightdm/session-issues]

2380. By Michael Terry on 2016-07-26

Whoops, add new tests to master list of tests

[Robert Ancell (robert-ancell)]

I've looked at the first issue (the segfault) in revision 2378. This turned out to be a reference count issue, I've taken your test and fixed in in trunk revision 2382.

[Robert Ancell (robert-ancell)]

Fixed the second issue in trunk r2384. When the session is accepted ownership is now taken from the greeter instead of of leaving the reference behind.

[Robert Ancell (robert-ancell)]

Merged in spirit, if not all the code is the same :)

[Robert Ancell (robert-ancell)]

Thanks!

=== modified file 'src/greeter.c'
--- src/greeter.c	2016-07-14 04:52:08 +0000
+++ src/greeter.c	2016-07-26 18:38:16 +0000
@@ -441,6 +441,16 @@
 }
 
 static void
+session_stopped_cb (Session *session, Greeter *greeter)
+{
+    if (session == greeter->priv->authentication_session)
+    {
+        g_signal_handlers_disconnect_matched (session, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, greeter);
+        g_clear_object (&greeter->priv->authentication_session);
+    }
+}
+
+static void
 reset_session (Greeter *greeter)
 {
     g_free (greeter->priv->remote_session);
@@ -448,7 +458,8 @@
     if (greeter->priv->authentication_session)
     {
         g_signal_handlers_disconnect_matched (greeter->priv->authentication_session, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, greeter);
-        session_stop (greeter->priv->authentication_session);
+        if (!session_get_is_run (greeter->priv->authentication_session))
+            session_stop (greeter->priv->authentication_session);
         g_clear_object (&greeter->priv->authentication_session);
     }
 
@@ -486,6 +497,7 @@
 
     g_signal_connect (G_OBJECT (greeter->priv->authentication_session), SESSION_SIGNAL_GOT_MESSAGES, G_CALLBACK (pam_messages_cb), greeter);
     g_signal_connect (G_OBJECT (greeter->priv->authentication_session), SESSION_SIGNAL_AUTHENTICATION_COMPLETE, G_CALLBACK (authentication_complete_cb), greeter);
+    g_signal_connect (G_OBJECT (greeter->priv->authentication_session), SESSION_SIGNAL_STOPPED, G_CALLBACK (session_stopped_cb), greeter);
 
     /* Use non-interactive service for autologin user */
     autologin_username = g_hash_table_lookup (greeter->priv->hints, "autologin-user");
@@ -590,6 +602,7 @@
     {
         g_signal_connect (G_OBJECT (greeter->priv->authentication_session), SESSION_SIGNAL_GOT_MESSAGES, G_CALLBACK (pam_messages_cb), greeter);
         g_signal_connect (G_OBJECT (greeter->priv->authentication_session), SESSION_SIGNAL_AUTHENTICATION_COMPLETE, G_CALLBACK (authentication_complete_cb), greeter);
+        g_signal_connect (G_OBJECT (greeter->priv->authentication_session), SESSION_SIGNAL_STOPPED, G_CALLBACK (session_stopped_cb), greeter);
 
         /* Run the session process */
         session_set_pam_service (greeter->priv->authentication_session, service);

=== modified file 'tests/Makefile.am'
--- tests/Makefile.am	2016-07-14 03:12:25 +0000
+++ tests/Makefile.am	2016-07-26 18:38:16 +0000
@@ -171,6 +171,8 @@
 	test-session-greeter \
 	test-session-greeter-autologin \
 	test-session-greeter-reconnect \
+	test-session-greeter-switch \
+	test-session-greeter-twice \
 	test-session-greeter-unlock \
 	test-vnc-login \
 	test-vnc-command \

=== added file 'tests/data/sessions/greeter-mir.desktop'
--- tests/data/sessions/greeter-mir.desktop	1970-01-01 00:00:00 +0000
+++ tests/data/sessions/greeter-mir.desktop	2016-07-26 18:38:16 +0000
@@ -0,0 +1,6 @@
+[Desktop Entry]
+Name=Test Session with greeter
+Comment=LightDM test Mir session that can run a greeter inside
+Exec=test-session
+X-LightDM-Allow-Greeter=true
+X-LightDM-Session-Type=mir

=== added file 'tests/scripts/session-greeter-switch.conf'
--- tests/scripts/session-greeter-switch.conf	1970-01-01 00:00:00 +0000
+++ tests/scripts/session-greeter-switch.conf	2016-07-26 18:38:16 +0000
@@ -0,0 +1,74 @@
+#
+# Check can unlock separate user with an in-session greeter
+#
+
+[Seat:*]
+user-session=greeter-mir
+
+#?*START-DAEMON
+#?RUNNER DAEMON-START
+
+# X server starts
+#?XSERVER-0 START VT=7 SEAT=seat0
+
+# Daemon connects when X server is ready
+#?*XSERVER-0 INDICATE-READY
+#?XSERVER-0 INDICATE-READY
+#?XSERVER-0 ACCEPT-CONNECT
+
+# Greeter starts
+#?GREETER-X-0 START XDG_SEAT=seat0 XDG_VTNR=7 XDG_SESSION_CLASS=greeter
+#?LOGIN1 ACTIVATE-SESSION SESSION=c0
+#?XSERVER-0 ACCEPT-CONNECT
+#?GREETER-X-0 CONNECT-XSERVER
+#?GREETER-X-0 CONNECT-TO-DAEMON
+#?GREETER-X-0 CONNECTED-TO-DAEMON
+
+# Attempt to log into account
+#?*GREETER-X-0 AUTHENTICATE USERNAME=no-password1
+#?GREETER-X-0 AUTHENTICATION-COMPLETE USERNAME=no-password1 AUTHENTICATED=TRUE
+#?*GREETER-X-0 START-SESSION
+
+# System compositor starts
+#?UNITY-SYSTEM-COMPOSITOR START FILE=/run/mir_socket VT=8 XDG_VTNR=8
+#?*UNITY-SYSTEM-COMPOSITOR READY
+
+# Switch to system compositor
+#?VT ACTIVATE VT=8
+
+# Greeter terminates
+#?GREETER-X-0 TERMINATE SIGNAL=15
+#?XSERVER-0 TERMINATE SIGNAL=15
+
+# Session starts
+#?SESSION-MIR-session-0 START XDG_SEAT=seat0 XDG_VTNR=8 XDG_GREETER_DATA_DIR=.*/no-password1 XDG_SESSION_TYPE=mir XDG_SESSION_DESKTOP=greeter-mir USER=no-password1
+#?LOGIN1 ACTIVATE-SESSION SESSION=c1
+#?UNITY-SYSTEM-COMPOSITOR SET-ACTIVE-SESSION ID=session-0
+
+# Start greeter inside session
+#?*SESSION-MIR-session-0 GREETER-START
+#?SESSION-MIR-session-0 GREETER-STARTED
+
+# Log into our account
+#?*SESSION-MIR-session-0 GREETER-AUTHENTICATE USERNAME=have-password1
+#?SESSION-MIR-session-0 GREETER-SHOW-PROMPT TEXT="Password:"
+#?*SESSION-MIR-session-0 GREETER-RESPOND TEXT="password"
+#?SESSION-MIR-session-0 GREETER-AUTHENTICATION-COMPLETE USERNAME=have-password1 AUTHENTICATED=TRUE
+#?*SESSION-MIR-session-0 GREETER-START-SESSION
+
+# And immediately re-try auth from first session
+#?*SESSION-MIR-session-0 GREETER-AUTHENTICATE USERNAME=have-password1
+#?SESSION-MIR-session-0 GREETER-SHOW-PROMPT TEXT="Password:"
+
+# New session starts
+#?SESSION-MIR-session-1 START XDG_SEAT=seat0 XDG_VTNR=8 XDG_GREETER_DATA_DIR=.*/have-password1 XDG_SESSION_TYPE=mir XDG_SESSION_DESKTOP=greeter-mir USER=have-password1
+#?UNITY-SYSTEM-COMPOSITOR SET-ACTIVE-SESSION ID=session-1
+#?LOGIN1 LOCK-SESSION SESSION=c1
+#?LOGIN1 ACTIVATE-SESSION SESSION=c2
+
+# Cleanup
+#?*STOP-DAEMON
+#?SESSION-MIR-session-0 TERMINATE SIGNAL=15
+#?SESSION-MIR-session-1 TERMINATE SIGNAL=15
+#?UNITY-SYSTEM-COMPOSITOR TERMINATE SIGNAL=15
+#?RUNNER DAEMON-EXIT STATUS=0

=== added file 'tests/scripts/session-greeter-twice.conf'
--- tests/scripts/session-greeter-twice.conf	1970-01-01 00:00:00 +0000
+++ tests/scripts/session-greeter-twice.conf	2016-07-26 18:38:16 +0000
@@ -0,0 +1,54 @@
+#
+# Check can unlock user with an in-session greeter and re-lock again
+#
+
+[Seat:*]
+autologin-user=have-password1
+user-session=greeter
+
+#?*START-DAEMON
+#?RUNNER DAEMON-START
+
+# X server starts
+#?XSERVER-0 START VT=7 SEAT=seat0
+
+# Daemon connects when X server is ready
+#?*XSERVER-0 INDICATE-READY
+#?XSERVER-0 INDICATE-READY
+#?XSERVER-0 ACCEPT-CONNECT
+
+# Session starts
+#?SESSION-X-0 START XDG_SEAT=seat0 XDG_VTNR=7 XDG_GREETER_DATA_DIR=.*/have-password1 XDG_SESSION_TYPE=x11 XDG_SESSION_DESKTOP=greeter USER=have-password1
+#?LOGIN1 ACTIVATE-SESSION SESSION=c0
+#?XSERVER-0 ACCEPT-CONNECT
+#?SESSION-X-0 CONNECT-XSERVER
+
+# Start greeter inside session
+#?*SESSION-X-0 GREETER-START
+#?SESSION-X-0 GREETER-STARTED
+
+# Log into our account
+#?*SESSION-X-0 GREETER-AUTHENTICATE USERNAME=have-password1
+#?SESSION-X-0 GREETER-SHOW-PROMPT TEXT="Password:"
+#?*SESSION-X-0 GREETER-RESPOND TEXT="password"
+#?SESSION-X-0 GREETER-AUTHENTICATION-COMPLETE USERNAME=have-password1 AUTHENTICATED=TRUE
+#?*SESSION-X-0 GREETER-START-SESSION
+
+# We are reactivated
+#?LOGIN1 ACTIVATE-SESSION SESSION=c0
+
+# Authenticate again
+#?*SESSION-X-0 GREETER-AUTHENTICATE USERNAME=have-password1
+#?SESSION-X-0 GREETER-SHOW-PROMPT TEXT="Password:"
+#?*SESSION-X-0 GREETER-RESPOND TEXT="password"
+#?SESSION-X-0 GREETER-AUTHENTICATION-COMPLETE USERNAME=have-password1 AUTHENTICATED=TRUE
+#?*SESSION-X-0 GREETER-START-SESSION
+
+# We are reactivated
+#?LOGIN1 ACTIVATE-SESSION SESSION=c0
+
+# Cleanup
+#?*STOP-DAEMON
+#?SESSION-X-0 TERMINATE SIGNAL=15
+#?XSERVER-0 TERMINATE SIGNAL=15
+#?RUNNER DAEMON-EXIT STATUS=0

=== added file 'tests/test-session-greeter-switch'
--- tests/test-session-greeter-switch	1970-01-01 00:00:00 +0000
+++ tests/test-session-greeter-switch	2016-07-26 18:38:16 +0000
@@ -0,0 +1,2 @@
+#!/bin/sh
+./src/dbus-env ./src/test-runner session-greeter-switch test-gobject-greeter

=== added file 'tests/test-session-greeter-twice'
--- tests/test-session-greeter-twice	1970-01-01 00:00:00 +0000
+++ tests/test-session-greeter-twice	2016-07-26 18:38:16 +0000
@@ -0,0 +1,2 @@
+#!/bin/sh
+./src/dbus-env ./src/test-runner session-greeter-twice test-gobject-greeter