From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 4 Nov 2024 10:18:29 +0000 (+0100)
Subject: sd-daemon: add fd array size safety check to sd_notify_with_fds()
X-Git-Tag: v257-rc1~31^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F35011%2Fhead;p=thirdparty%2Fsystemd.git

sd-daemon: add fd array size safety check to sd_notify_with_fds()

The previous commit removed the UINT_MAX check for the fd array. Let's
now re-add one, but at a better place, and with a more useful limit. As
it turns out the kernel does not allow passing more than 253 fds at the
same time, hence use that as limit. And do so immediately before
calculating the control buffer size, so that we catch multiplication
overflows.
---

diff --git a/man/sd_notify.xml b/man/sd_notify.xml
index 5e6c9d6dbcd..0c993e31f77 100644
--- a/man/sd_notify.xml
+++ b/man/sd_notify.xml
@@ -499,6 +499,26 @@
     successfully. Specifically, no error is returned when a file descriptor is attempted to be stored using
     <varname>FDSTORE=1</varname> but the service is not actually configured to permit storing of file
     descriptors (see above).</para>
+
+    <refsect2 id='errors'>
+      <title>Errors</title>
+
+      <para>Returned errors may indicate the following problems:</para>
+
+      <variablelist>
+        <varlistentry>
+          <term><constant>-E2BIG</constant></term>
+
+          <listitem><para>More file descriptors passed at once than the system allows. On Linux the number of
+          file descriptors that may be passed across <constant>AF_UNIX</constant> sockets at once is 253, see
+          <citerefentry
+          project='man-pages'><refentrytitle>unix</refentrytitle><manvolnum>7</manvolnum></citerefentry> for
+          details.</para>
+
+          <xi:include href="version-info.xml" xpointer="v257"/></listitem>
+        </varlistentry>
+      </variablelist>
+    </refsect2>
   </refsect1>
 
   <refsect1>
diff --git a/src/basic/missing_socket.h b/src/basic/missing_socket.h
index 55de40a3216..8460ce13bf3 100644
--- a/src/basic/missing_socket.h
+++ b/src/basic/missing_socket.h
@@ -68,3 +68,10 @@
 #ifndef IPV6_RECVFRAGSIZE
 #define IPV6_RECVFRAGSIZE 77
 #endif
+
+/* The maximum number of fds that SCM_RIGHTS accepts. This is an internal kernel constant, but very much
+ * useful for userspace too. It's documented in unix(7) these days, hence should be fairly reliable to define
+ * here. */
+#ifndef SCM_MAX_FD
+#define SCM_MAX_FD 253U
+#endif
diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c
index faf45c27509..4d3d678cb45 100644
--- a/src/libsystemd/sd-daemon/sd-daemon.c
+++ b/src/libsystemd/sd-daemon/sd-daemon.c
@@ -472,6 +472,12 @@ static int pid_notify_with_fds_internal(
         assert_return(state, -EINVAL);
         assert_return(fds || n_fds == 0, -EINVAL);
 
+        /* Let's make sure the multiplications below don't overflow, and also return a recognizable error in
+         * case the caller tries to send more fds than the kernel limit. The kernel would return EINVAL which
+         * is not too useful I'd say. */
+        if (n_fds > SCM_MAX_FD)
+                return -E2BIG;
+
         const char *e = getenv("NOTIFY_SOCKET");
         if (!e)
                 return 0;
diff --git a/src/notify/notify.c b/src/notify/notify.c
index 8937457ec99..6afb9560c65 100644
--- a/src/notify/notify.c
+++ b/src/notify/notify.c
@@ -453,6 +453,8 @@ static int run(int argc, char* argv[]) {
                 r = sd_pid_notify_with_fds(arg_pid.pid, /* unset_environment= */ false, msg, a, k);
 
         }
+        if (r == -E2BIG)
+                return log_error_errno(r, "Too many file descriptors passed.");
         if (r < 0)
                 return log_error_errno(r, "Failed to notify init system: %m");
         if (r == 0)