logind: fail on CreateSession if already in session

Right now, if you're already in a session and call CreateSession, we
return information about the current session of yours. This is highy
confusing and a nasty hack. Avoid that, and instead return a commonly
known error, so the caller can detect that.

This has the side-effect, that we no longer override XDG_VTNR and XDG_SEAT
in pam_systemd, if you're already in a session. But this sounds like the
right thing to do, anyway.

Author: David Herrmann

src/libsystemd/sd-bus/bus-common-errors.h

@@ -58,6 +58,7 @@
 #define BUS_ERROR_DEVICE_NOT_TAKEN "org.freedesktop.login1.DeviceNotTaken"
 #define BUS_ERROR_OPERATION_IN_PROGRESS "org.freedesktop.login1.OperationInProgress"
 #define BUS_ERROR_SLEEP_VERB_NOT_SUPPORTED "org.freedesktop.login1.SleepVerbNotSupported"
+#define BUS_ERROR_SESSION_BUSY "org.freedesktop.login1.SessionBusy"
 
 #define BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED "org.freedesktop.timedate1.AutomaticTimeSyncEnabled"
 

src/login/logind-core.c

@@ -317,7 +317,6 @@ int manager_get_session_by_pid(Manager *m, pid_t pid, Session **session) {
         int r;
 
         assert(m);
-        assert(session);
 
         if (pid < 1)
                 return -EINVAL;
@@ -330,7 +329,8 @@ int manager_get_session_by_pid(Manager *m, pid_t pid, Session **session) {
         if (!s)
                 return 0;
 
-        *session = s;
+        if (session)
+                *session = s;
         return 1;
 }
 

src/login/logind-dbus.c

@@ -689,58 +689,23 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus
                         return r;
         }
 
-        manager_get_session_by_pid(m, leader, &session);
-        if (!session && vtnr > 0 && vtnr < m->seat0->position_count) {
-                session = m->seat0->positions[vtnr];
-                /*
-                 * Old gdm and lightdm start the user-session on the same VT as
-                 * the greeter session. But they destroy the greeter session
-                 * after the user-session and want the user-session to take
-                 * over the VT. We need to support this for
-                 * backwards-compatibility, so make sure we allow new sessions
-                 * on a VT that a greeter is running on.
-                 */
-                if (session && session->class == SESSION_GREETER)
-                        session = NULL;
-        }
-        if (session) {
-                _cleanup_free_ char *path = NULL;
-                _cleanup_close_ int fifo_fd = -1;
-
-                /* Session already exists, client is probably
-                 * something like "su" which changes uid but is still
-                 * the same session */
-
-                fifo_fd = session_create_fifo(session);
-                if (fifo_fd < 0)
-                        return fifo_fd;
-
-                path = session_bus_path(session);
-                if (!path)
-                        return -ENOMEM;
-
-                log_debug("Sending reply about an existing session: "
-                          "id=%s object_path=%s uid=%u runtime_path=%s "
-                          "session_fd=%d seat=%s vtnr=%u",
-                          session->id,
-                          path,
-                          (uint32_t) session->user->uid,
-                          session->user->runtime_path,
-                          fifo_fd,
-                          session->seat ? session->seat->id : "",
-                          (uint32_t) session->vtnr);
-
-                return sd_bus_reply_method_return(
-                                message, "soshusub",
-                                session->id,
-                                path,
-                                session->user->runtime_path,
-                                fifo_fd,
-                                (uint32_t) session->user->uid,
-                                session->seat ? session->seat->id : "",
-                                (uint32_t) session->vtnr,
-                                true);
-        }
+        r = manager_get_session_by_pid(m, leader, NULL);
+        if (r > 0)
+                return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY, "Already running in a session");
+
+        /*
+         * Old gdm and lightdm start the user-session on the same VT as
+         * the greeter session. But they destroy the greeter session
+         * after the user-session and want the user-session to take
+         * over the VT. We need to support this for
+         * backwards-compatibility, so make sure we allow new sessions
+         * on a VT that a greeter is running on.
+         */
+        if (vtnr > 0 &&
+            vtnr < m->seat0->position_count &&
+            m->seat0->positions[vtnr] &&
+            m->seat0->positions[vtnr]->class != SESSION_GREETER)
+                return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY, "Already occupied by a session");
 
         audit_session_from_pid(leader, &audit_id);
         if (audit_id > 0) {

src/login/pam_systemd.c

@@ -31,6 +31,7 @@
 #include <security/pam_ext.h>
 #include <security/pam_misc.h>
 
+#include "bus-common-errors.h"
 #include "util.h"
 #include "audit.h"
 #include "macro.h"
@@ -399,8 +400,13 @@ _public_ PAM_EXTERN int pam_sm_open_session(
                                remote_host,
                                0);
         if (r < 0) {
-                pam_syslog(handle, LOG_ERR, "Failed to create session: %s", bus_error_message(&error, r));
-                return PAM_SYSTEM_ERR;
+                if (sd_bus_error_has_name(&error, BUS_ERROR_SESSION_BUSY)) {
+                        pam_syslog(handle, LOG_DEBUG, "Cannot create session: %s", bus_error_message(&error, r));
+                        return PAM_SUCCESS;
+                } else {
+                        pam_syslog(handle, LOG_ERR, "Failed to create session: %s", bus_error_message(&error, r));
+                        return PAM_SYSTEM_ERR;
+                }
         }
 
         r = sd_bus_message_read(reply,