notify: beef up --pid= logic

Prompted by the discussions on #15547.

man/systemd-notify.xml

@@ -84,12 +84,13 @@
       <varlistentry>
         <term><option>--pid=</option></term>
 
-        <listitem><para>Inform the init system about the main PID of
-        the daemon. Takes a PID as argument. If the argument is
-        omitted, the PID of the process that invoked
-        <command>systemd-notify</command> is used. This is equivalent
-        to <command>systemd-notify MAINPID=$PID</command>. For details
-        about the semantics of this option see
+        <listitem><para>Inform the service manager about the main PID of the daemon. Takes a PID as
+        argument. If the argument is specified as <literal>auto</literal> or omitted, the PID of the process
+        that invoked <command>systemd-notify</command> is used, except if that's the service manager. If the
+        argument is specified as <literal>self</literal>, the PID of the <command>systemd-notify</command>
+        command itself is used, and if <literal>parent</literal> is specified the calling process' PID is
+        used — even if it is the service manager. This is equivalent to <command>systemd-notify
+        MAINPID=$PID</command>. For details about the semantics of this option see
         <citerefentry><refentrytitle>sd_notify</refentrytitle><manvolnum>3</manvolnum></citerefentry>.</para></listitem>
       </varlistentry>
 

src/basic/efivars.c

@@ -313,6 +313,29 @@ int systemd_efi_options_variable(char **line) {
                 return 0;
         }
 
+        /* In SecureBoot mode this is probably not what you want. As your cmdline is cryptographically signed
+         * like when using Type #2 EFI Unified Kernel Images (https://systemd.io/BOOT_LOADER_SPECIFICATION/)
+         * The user's intention is then that the cmdline should not be modified. You want to make sure that
+         * the system starts up as exactly specified in the signed artifact.
+         *
+         * (NB: to make testing purposes we still check the $SYSTEMD_EFI_OPTIONS env var above, even when in
+         * SecureBoot mode.) */
+        if (is_efi_secure_boot()) {
+                _cleanup_free_ char *k;
+
+                k = efi_variable_path(EFI_VENDOR_SYSTEMD, "SystemdOptions");
+                if (!k)
+                        return -ENOMEM;
+
+                /* Let's be helpful with the returned error and check if the variable exists at all. If it
+                 * does, let's return a recognizable error (EPERM), and if not ENODATA. */
+
+                if (access(k, F_OK) < 0)
+                        return errno == -ENOENT ? -ENODATA : -errno;
+
+                return -EPERM;
+        }
+
         r = efi_get_variable_string(EFI_VENDOR_SYSTEMD, "SystemdOptions", line);
         if (r == -ENOENT)
                 return -ENODATA;

src/basic/proc-cmdline.c

@@ -39,18 +39,6 @@ int proc_cmdline(char **ret) {
                 return read_one_line_file("/proc/cmdline", ret);
 }
 
-/* In SecureBoot mode this is probably not what you want. As your cmdline is
- * cryptographically signed like when using Type #2 EFI Unified Kernel Images
- * (https://systemd.io/BOOT_LOADER_SPECIFICATION/) The user's intention is then
- * that the cmdline should not be modified.  You want to make sure that the
- * system starts up as exactly specified in the signed artifact. */
-static int systemd_options_variable(char **line) {
-        if (is_efi_secure_boot())
-                return -ENODATA;
-
-        return systemd_efi_options_variable(line);
-}
-
 static int proc_cmdline_extract_first(const char **p, char **ret_word, ProcCmdlineFlags flags) {
         const char *q = *p;
         int r;
@@ -131,7 +119,7 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF
 
         /* We parse the EFI variable first, because later settings have higher priority. */
 
-        r = systemd_options_variable(&line);
+        r = systemd_efi_options_variable(&line);
         if (r < 0 && r != -ENODATA)
                 log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m");
 
@@ -262,7 +250,7 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val
                 return r;
 
         line = mfree(line);
-        r = systemd_options_variable(&line);
+        r = systemd_efi_options_variable(&line);
         if (r == -ENODATA)
                 return false; /* Not found */
         if (r < 0)

src/notify/notify.c

@@ -54,6 +54,26 @@ static int help(void) {
         return 0;
 }
 
+static pid_t manager_pid(void) {
+        const char *e;
+        pid_t pid;
+        int r;
+
+        /* If we run as a service managed by systemd --user the $MANAGERPID environment variable points to
+         * the service manager's PID. */
+        e = getenv("MANAGERPID");
+        if (!e)
+                return 0;
+
+        r = parse_pid(e, &pid);
+        if (r < 0) {
+                log_warning_errno(r, "$MANAGERPID is set to an invalid PID, ignoring: %s", e);
+                return 0;
+        }
+
+        return pid;
+}
+
 static int parse_argv(int argc, char *argv[]) {
 
         enum {
@@ -96,14 +116,25 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_PID:
-
-                        if (optarg) {
-                                if (parse_pid(optarg, &arg_pid) < 0)
-                                        return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                                               "Failed to parse PID %s.", optarg);
-                        } else
+                        if (isempty(optarg) || streq(optarg, "auto")) {
                                 arg_pid = getppid();
 
+                                if (arg_pid <= 1 ||
+                                    arg_pid == manager_pid()) /* Don't send from PID 1 or the service
+                                                               * manager's PID (which might be distinct from
+                                                               * 1, if we are a --user instance), that'd just
+                                                               * be confusing for the service manager */
+                                        arg_pid = getpid();
+                        } else if (streq(optarg, "parent"))
+                                arg_pid = getppid();
+                        else if (streq(optarg, "self"))
+                                arg_pid = getpid();
+                        else {
+                                r = parse_pid(optarg, &arg_pid);
+                                if (r < 0)
+                                        return log_error_errno(r, "Failed to parse PID %s.", optarg);
+                        }
+
                         break;
 
                 case ARG_STATUS:
@@ -151,6 +182,7 @@ static int run(int argc, char* argv[]) {
         _cleanup_strv_free_ char **final_env = NULL;
         char* our_env[4];
         unsigned i = 0;
+        pid_t source_pid;
         int r;
 
         log_show_color(true);
@@ -207,7 +239,18 @@ static int run(int argc, char* argv[]) {
             setreuid(arg_uid, (uid_t) -1) < 0)
                 return log_error_errno(errno, "Failed to change UID: %m");
 
-        r = sd_pid_notify(arg_pid ? arg_pid : getppid(), false, n);
+        if (arg_pid > 0)
+                source_pid = arg_pid;
+        else {
+                /* Pretend the message originates from our parent, given that we are typically called from a
+                 * shell script, i.e. we are not the main process of a service but only a child of it. */
+                source_pid = getppid();
+                if (source_pid <= 1 ||
+                    source_pid == manager_pid()) /* safety check: don't claim we'd send anything from PID 1
+                                                  * or the service manager itself */
+                        source_pid = 0;
+        }
+        r = sd_pid_notify(source_pid, false, n);
         if (r < 0)
                 return log_error_errno(r, "Failed to notify init system: %m");
         if (r == 0)