Merge pull request #34124 from YHNdnzj/socket-accept-name

core: honor FileDescriptorName= too for Accept=yes sockets, plus several other cleanups

NEWS

@@ -31,6 +31,10 @@ CHANGES WITH 257 in spe:
           by default when combined with --scope, will be changed in a future
           release to be enabled by default.
 
+        * The FileDescriptorName= setting for socket units is now honored by
+          Accept=yes sockets too, where it was previously silently ignored and
+          "connection" was used unconditionally.
+
         * systemd-logind now always obeys inhibitor locks, where previously it
           ignored locks taken by the caller or when the caller was root. A
           privileged caller can always close the other sessions, remove the

man/systemd.socket.xml

@@ -853,18 +853,15 @@
 
       <varlistentry>
         <term><varname>FileDescriptorName=</varname></term>
-        <listitem><para>Assigns a name to all file descriptors this
+        <listitem><para>Assigns a name to all file descriptors this socket unit encapsulates.
-        socket unit encapsulates. This is useful to help activated
+        This is useful to help activated services identify specific file descriptors, if multiple fds are passed.
-        services identify specific file descriptors, if multiple fds
+        Services may use the
-        are passed. Services may use the
         <citerefentry><refentrytitle>sd_listen_fds_with_names</refentrytitle><manvolnum>3</manvolnum></citerefentry>
-        call to acquire the names configured for the received file
+        call to acquire the names configured for the received file descriptors. Names may contain any ASCII character,
-        descriptors. Names may contain any ASCII character, but must
+        but must exclude control characters and <literal>:</literal>, and must be at most 255 characters in length.
-        exclude control characters and <literal>:</literal>, and must
+        If this setting is not used, the file descriptor name defaults to the name of the socket unit
-        be at most 255 characters in length. If this setting is not
+        (including its <filename>.socket</filename> suffix) when <varname>Accept=no</varname>,
-        used, the file descriptor name defaults to the name of the
+        <literal>connection</literal> otherwise.</para>
-        socket unit, including its <filename>.socket</filename>
-        suffix.</para>
 
         <xi:include href="version-info.xml" xpointer="v227"/></listitem>
       </varlistentry>

src/core/mount.c

@@ -574,9 +574,6 @@ static int mount_verify(Mount *m) {
         if (p && !p->what && !UNIT(m)->perpetual)
                 return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC), "What= setting is missing. Refusing.");
 
-        if (m->exec_context.pam_name && m->kill_context.kill_mode != KILL_CONTROL_GROUP)
-                return log_unit_error_errno(UNIT(m), SYNTHETIC_ERRNO(ENOEXEC), "Unit has PAM enabled. Kill mode must be set to control-group'. Refusing.");
-
         return 0;
 }
 

src/core/service.c

@@ -683,9 +683,6 @@ static int service_verify(Service *s) {
         if (s->type == SERVICE_DBUS && !s->bus_name)
                 return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Service is of type D-Bus but no D-Bus service name has been specified. Refusing.");
 
-        if (s->exec_context.pam_name && !IN_SET(s->kill_context.kill_mode, KILL_CONTROL_GROUP, KILL_MIXED))
-                return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Service has PAM enabled. Kill mode must be set to 'control-group' or 'mixed'. Refusing.");
-
         if (s->usb_function_descriptors && !s->usb_function_strings)
                 log_unit_warning(UNIT(s), "Service has USBFunctionDescriptors= setting, but no USBFunctionStrings=. Ignoring.");
 
@@ -1429,6 +1426,7 @@ static int service_collect_fds(
         assert(n_storage_fds);
 
         if (s->socket_fd >= 0) {
+                Socket *sock = ASSERT_PTR(SOCKET(UNIT_DEREF(s->accept_socket)));
 
                 /* Pass the per-connection socket */
 
@@ -1436,7 +1434,7 @@ static int service_collect_fds(
                 if (!rfds)
                         return -ENOMEM;
 
-                rfd_names = strv_new("connection");
+                rfd_names = strv_new(socket_fdname(sock));
                 if (!rfd_names)
                         return -ENOMEM;
 

src/core/socket.c

@@ -328,7 +328,7 @@ static int socket_add_extras(Socket *s) {
 
         if (have_non_accept_socket(s)) {
 
-                if (!UNIT_DEREF(s->service)) {
+                if (!UNIT_ISSET(s->service)) {
                         Unit *x;
 
                         r = unit_load_related_unit(u, ".service", &x);
@@ -410,18 +410,15 @@ static int socket_verify(Socket *s) {
         if (!s->ports)
                 return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Unit has no Listen setting (ListenStream=, ListenDatagram=, ListenFIFO=, ...). Refusing.");
 
+        if (s->max_connections <= 0)
+                return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "MaxConnection= setting too small. Refusing.");
+
         if (s->accept && have_non_accept_socket(s))
                 return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Unit configured for accepting sockets, but sockets are non-accepting. Refusing.");
 
-        if (s->accept && s->max_connections <= 0)
+        if (s->accept && UNIT_ISSET(s->service))
-                return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "MaxConnection= setting too small. Refusing.");
-
-        if (s->accept && UNIT_DEREF(s->service))
                 return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Explicit service configuration for accepting socket units not supported. Refusing.");
 
-        if (s->exec_context.pam_name && s->kill_context.kill_mode != KILL_CONTROL_GROUP)
-                return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Unit has PAM enabled. Kill mode must be set to 'control-group'. Refusing.");
-
         if (!strv_isempty(s->symlinks) && !socket_find_symlink_target(s))
                 return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Unit has symlinks set but none or more than one node in the file system. Refusing.");
 
@@ -2396,10 +2393,9 @@ static void socket_enter_running(Socket *s, int cfd_in) {
                 s->n_accepted++;
 
                 r = service_set_socket_fd(SERVICE(service), cfd, s, p, s->selinux_context_from_net);
+                if (ERRNO_IS_NEG_DISCONNECT(r))
+                        return;
                 if (r < 0) {
-                        if (ERRNO_IS_DISCONNECT(r))
-                                return;
-
                         log_unit_warning_errno(UNIT(s), r, "Failed to set socket on service: %m");
                         goto fail;
                 }
@@ -3422,17 +3418,22 @@ static int socket_get_timeout(Unit *u, usec_t *timeout) {
         return 1;
 }
 
-char* socket_fdname(Socket *s) {
+const char* socket_fdname(Socket *s) {
         assert(s);
 
-        /* Returns the name to use for $LISTEN_NAMES. If the user
+        /* Returns the name to use for $LISTEN_FDNAMES. If the user didn't specify anything specifically,
-         * didn't specify anything specifically, use the socket unit's
+         * use the socket unit's name as fallback for Accept=no sockets, "connection" otherwise. */
-         * name as fallback. */
 
-        return s->fdname ?: UNIT(s)->id;
+        if (s->fdname)
+                return s->fdname;
+
+        if (s->accept)
+                return "connection";
+
+        return UNIT(s)->id;
 }
 
-static PidRef *socket_control_pid(Unit *u) {
+static PidRef* socket_control_pid(Unit *u) {
         return &ASSERT_PTR(SOCKET(u))->control_pid;
 }
 

src/core/socket.h

@@ -185,7 +185,7 @@ int socket_port_to_address(const SocketPort *s, char **ret);
 
 int socket_load_service_unit(Socket *s, int cfd, Unit **ret);
 
-char* socket_fdname(Socket *s);
+const char* socket_fdname(Socket *s);
 
 extern const UnitVTable socket_vtable;
 

src/core/swap.c

@@ -256,9 +256,6 @@ static int swap_verify(Swap *s) {
         if (!unit_has_name(UNIT(s), e))
                 return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Value of What= and unit name do not match, not loading.");
 
-        if (s->exec_context.pam_name && s->kill_context.kill_mode != KILL_CONTROL_GROUP)
-                return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Unit has PAM enabled. Kill mode must be set to 'control-group'. Refusing to load.");
-
         return 0;
 }
 

src/core/unit.c

@@ -4216,9 +4216,10 @@ static int user_from_unit_name(Unit *u, char **ret) {
         return 0;
 }
 
-static int unit_verify_contexts(const Unit *u, const ExecContext *ec) {
+static int unit_verify_contexts(const Unit *u) {
         assert(u);
 
+        const ExecContext *ec = unit_get_exec_context(u);
         if (!ec)
                 return 0;
 
@@ -4232,6 +4233,11 @@ static int unit_verify_contexts(const Unit *u, const ExecContext *ec) {
             exec_needs_mount_namespace(ec, /* params = */ NULL, /* runtime = */ NULL))
                 return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOEXEC), "WorkingDirectory= may not be below /proc/, /sys/ or /dev/ when using mount namespacing. Refusing.");
 
+        const KillContext *kc = unit_get_kill_context(u);
+
+        if (ec->pam_name && kc && !IN_SET(kc->kill_mode, KILL_CONTROL_GROUP, KILL_MIXED))
+                return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOEXEC), "Unit has PAM enabled. Kill mode must be set to 'control-group' or 'mixed'. Refusing.");
+
         return 0;
 }
 
@@ -4362,7 +4368,7 @@ int unit_patch_contexts(Unit *u) {
                 }
         }
 
-        return unit_verify_contexts(u, ec);
+        return unit_verify_contexts(u);
 }
 
 ExecContext *unit_get_exec_context(const Unit *u) {

src/core/unit.h

@@ -926,15 +926,15 @@ void unit_ref_unset(UnitRef *ref);
 
 int unit_patch_contexts(Unit *u);
 
-ExecContext *unit_get_exec_context(const Unit *u) _pure_;
+ExecContext* unit_get_exec_context(const Unit *u) _pure_;
-KillContext *unit_get_kill_context(const Unit *u) _pure_;
+KillContext* unit_get_kill_context(const Unit *u) _pure_;
-CGroupContext *unit_get_cgroup_context(const Unit *u) _pure_;
+CGroupContext* unit_get_cgroup_context(const Unit *u) _pure_;
 
-ExecRuntime *unit_get_exec_runtime(const Unit *u) _pure_;
+ExecRuntime* unit_get_exec_runtime(const Unit *u) _pure_;
-CGroupRuntime *unit_get_cgroup_runtime(const Unit *u) _pure_;
+CGroupRuntime* unit_get_cgroup_runtime(const Unit *u) _pure_;
 
 int unit_setup_exec_runtime(Unit *u);
-CGroupRuntime *unit_setup_cgroup_runtime(Unit *u);
+CGroupRuntime* unit_setup_cgroup_runtime(Unit *u);
 
 const char* unit_escape_setting(const char *s, UnitWriteFlags flags, char **buf);
 char* unit_concat_strv(char **l, UnitWriteFlags flags);

units/systemd-bootctl@.service

@@ -16,5 +16,4 @@ After=local-fs.target
 Before=shutdown.target
 
 [Service]
-Environment=LISTEN_FDNAMES=varlink
 ExecStart=bootctl

units/systemd-creds@.service

@@ -15,5 +15,4 @@ Conflicts=shutdown.target initrd-switch-root.target
 Before=shutdown.target initrd-switch-root.target
 
 [Service]
-Environment=LISTEN_FDNAMES=varlink
 ExecStart=-systemd-creds

units/systemd-pcrextend@.service.in

@@ -16,5 +16,4 @@ Conflicts=shutdown.target initrd-switch-root.target
 Before=shutdown.target initrd-switch-root.target
 
 [Service]
-Environment=LISTEN_FDNAMES=varlink
 ExecStart=-{{LIBEXECDIR}}/systemd-pcrextend

units/systemd-pcrlock@.service.in

@@ -17,5 +17,4 @@ Before=sysinit.target shutdown.target
 After=systemd-remount-fs.service var.mount
 
 [Service]
-Environment=LISTEN_FDNAMES=varlink
 ExecStart={{LIBEXECDIR}}/systemd-pcrlock --location=770

units/systemd-sysext@.service

@@ -16,5 +16,4 @@ Conflicts=shutdown.target initrd-switch-root.target
 Before=shutdown.target initrd-switch-root.target
 
 [Service]
-Environment=LISTEN_FDNAMES=varlink
 ExecStart=-systemd-sysext