run: do not pass the pty slave fd to transient service in a machine

Follow-up for 28459ba1f4df824d5ef7f7d1a9acb6953ea24045

The pty path returned by OpenMachinePTY() cannot be opened from outside
the machine, hence let's use the plain Standard{Input,Output,Error}=tty
in such a case. This means if --machine= is specified, #32916 would occur.
A comprehensive fix requires a new dbus method in machined, which shall
be material for v257.

See also: https://github.com/systemd/systemd/pull/33216#discussion_r1628020429

Replaces #33216

Co-authored-by: Mike Yuan <me@yhndnzj.com>

diff --git a/src/run/run.c b/src/run/run.c
index 368952b82f355d1d51a07bf57c00f4756946cebe..5779403b9c5723f0cb03dd55ddffc4a7628b95d8 100644 (file)
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -1073,7 +1073,7 @@ static int transient_kill_set_properties(sd_bus_message *m) {
         return 0;
 }
 
-static int transient_service_set_properties(sd_bus_message *m, const char *pty_path) {
+static int transient_service_set_properties(sd_bus_message *m, const char *pty_path, int pty_fd) {
         bool send_term = false;
         int r;
 
@@ -1083,6 +1083,7 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p
         bool use_ex_prop = arg_expand_environment == 0;
 
         assert(m);
+        assert(pty_path || pty_fd < 0);
 
         r = transient_unit_set_properties(m, UNIT_SERVICE, arg_property);
         if (r < 0)
@@ -1133,18 +1134,22 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p
         }
 
         if (pty_path) {
-                _cleanup_close_ int pty_slave = -EBADF;
-
-                pty_slave = open_terminal(pty_path, O_RDWR|O_NOCTTY|O_CLOEXEC);
-                if (pty_slave < 0)
-                        return pty_slave;
+                r = sd_bus_message_append(m, "(sv)", "TTYPath", "s", pty_path);
+                if (r < 0)
+                        return bus_log_create_error(r);
 
-                r = sd_bus_message_append(m,
-                                          "(sv)(sv)(sv)(sv)",
-                                          "StandardInputFileDescriptor", "h", pty_slave,
-                                          "StandardOutputFileDescriptor", "h", pty_slave,
-                                          "StandardErrorFileDescriptor", "h", pty_slave,
-                                          "TTYPath", "s", pty_path);
+                if (pty_fd >= 0)
+                        r = sd_bus_message_append(m,
+                                                  "(sv)(sv)(sv)",
+                                                  "StandardInputFileDescriptor", "h", pty_fd,
+                                                  "StandardOutputFileDescriptor", "h", pty_fd,
+                                                  "StandardErrorFileDescriptor", "h", pty_fd);
+                else
+                        r = sd_bus_message_append(m,
+                                                  "(sv)(sv)(sv)",
+                                                  "StandardInput", "s", "tty",
+                                                  "StandardOutput", "s", "tty",
+                                                  "StandardError", "s", "tty");
                 if (r < 0)
                         return bus_log_create_error(r);
 
@@ -1526,7 +1531,8 @@ static int make_transient_service_unit(
                 sd_bus *bus,
                 sd_bus_message **message,
                 const char *service,
-                const char *pty_path) {
+                const char *pty_path,
+                int pty_fd) {
 
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
         int r;
@@ -1553,7 +1559,7 @@ static int make_transient_service_unit(
         if (r < 0)
                 return bus_log_create_error(r);
 
-        r = transient_service_set_properties(m, pty_path);
+        r = transient_service_set_properties(m, pty_path, pty_fd);
         if (r < 0)
                 return r;
 
@@ -1675,7 +1681,7 @@ static int start_transient_service(sd_bus *bus) {
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL;
         _cleanup_free_ char *service = NULL, *pty_path = NULL;
-        _cleanup_close_ int master = -EBADF;
+        _cleanup_close_ int master = -EBADF, slave = -EBADF;
         int r;
 
         assert(bus);
@@ -1702,6 +1708,10 @@ static int start_transient_service(sd_bus *bus) {
                         if (unlockpt(master) < 0)
                                 return log_error_errno(errno, "Failed to unlock tty: %m");
 
+                        slave = open_terminal(pty_path, O_RDWR|O_NOCTTY|O_CLOEXEC);
+                        if (slave < 0)
+                                return log_error_errno(slave, "Failed to open pty slave: %m");
+
                 } else if (arg_transport == BUS_TRANSPORT_MACHINE) {
                         _cleanup_(sd_bus_unrefp) sd_bus *system_bus = NULL;
                         _cleanup_(sd_bus_message_unrefp) sd_bus_message *pty_reply = NULL;
@@ -1731,6 +1741,9 @@ static int start_transient_service(sd_bus *bus) {
                         pty_path = strdup(s);
                         if (!pty_path)
                                 return log_oom();
+
+                        // FIXME: Introduce OpenMachinePTYEx() that accepts ownership/permission as param
+                        // and additionally returns the pty fd, for #33216 and #32999
                 } else
                         assert_not_reached();
         }
@@ -1757,9 +1770,10 @@ static int start_transient_service(sd_bus *bus) {
                         return r;
         }
 
-        r = make_transient_service_unit(bus, &m, service, pty_path);
+        r = make_transient_service_unit(bus, &m, service, pty_path, slave);
         if (r < 0)
                 return r;
+        slave = safe_close(slave);
 
         polkit_agent_open_if_enabled(arg_transport, arg_ask_password);
 
@@ -2204,7 +2218,7 @@ static int make_transient_trigger_unit(
                 if (r < 0)
                         return bus_log_create_error(r);
 
-                r = transient_service_set_properties(m, NULL);
+                r = transient_service_set_properties(m, /* pty_path = */ NULL, /* pty_fd = */ -EBADF);
                 if (r < 0)
                         return r;