core: be stricter when handling PID files and MAINPID sd_notify() messagessubmit/tizen_5.0/20191230.043259submit/tizen_5.0/20191220.160949accepted/tizen/5.0/unified/20200106.075326tizen_5.0accepted/tizen_5.0_unified

Let's be more restrictive when validating PID files and MAINPID=
messages: don't accept PIDs that make no sense, and if the configuration
source is not trusted, don't accept out-of-cgroup PIDs. A configuratin
source is considered trusted when the PID file is owned by root, or the
message was received from root.

This should lock things down a bit, in case service authors write out
PID files from unprivileged code or use NotifyAccess=all with
unprivileged code. Note that doing so was always problematic, just now
it's a bit less problematic.

When we open the PID file we'll now use the CHASE_SAFE chase_symlinks()
logic, to ensure that we won't follow an unpriviled-owned symlink to a
privileged-owned file thinking this was a valid privileged PID file,
even though it really isn't.

Fixes: #6632
(cherry picked from commit db256aab13d8a89d583ecd2bacf0aca87c66effc)

Resolves: #1663143

Change-Id: Id91df4fb9e46224a12dc6c260a1c3b8a9e4cc553

8 files changed, 427 insertions, 56 deletions

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 875d368fcf..d3bf79691f 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -246,16 +246,15 @@
       <varlistentry>
         <term><varname>PIDFile=</varname></term>
 
-        <listitem><para>Takes an absolute file name pointing to the
-        PID file of this daemon. Use of this option is recommended for
-        services where <varname>Type=</varname> is set to
-        <option>forking</option>. systemd will read the PID of the
-        main process of the daemon after start-up of the service.
-        systemd will not write to the file configured here, although
-        it will remove the file after the service has shut down if it
-        still exists.
-        </para>
-        </listitem>
+        <listitem><para>Takes an absolute path referring to the PID file of the service. Usage of this option is
+        recommended for services where <varname>Type=</varname> is set to <option>forking</option>. The service manager
+        will read the PID of the main process of the service from this file after start-up of the service. The service
+        manager will not write to the file configured here, although it will remove the file after the service has shut
+        down if it still exists. The PID file does not need to be owned by a privileged user, but if it is owned by an
+        unprivileged user additional safety restrictions are enforced: the file may not be a symlink to a file owned by
+        a different user (neither directly nor indirectly), and the PID file must refer to a process already belonging
+        to the service.</para></listitem>
+
       </varlistentry>
 
       <varlistentry>
diff --git a/src/core/manager.c b/src/core/manager.c
index e8b1d0f52f..5abd531f6a 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1607,11 +1607,17 @@ static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, ui
         return 0;
 }
 
-static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, const char *buf, FDSet *fds) {
+static void manager_invoke_notify_message(
+                Manager *m,
+                Unit *u,
+                const struct ucred *ucred,
+                const char *buf,
+                FDSet *fds) {
         _cleanup_strv_free_ char **tags = NULL;
 
         assert(m);
         assert(u);
+        assert(ucred);
         assert(buf);
 
         tags = strv_split(buf, "\n\r");
@@ -1621,7 +1627,7 @@ static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, const
         }
 
         if (UNIT_VTABLE(u)->notify_message)
-                UNIT_VTABLE(u)->notify_message(u, pid, tags, fds);
+                UNIT_VTABLE(u)->notify_message(u, ucred, tags, fds);
         else
                 log_unit_debug(u, "Got notification message for unit. Ignoring.");
 }
@@ -1714,19 +1720,19 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
          * to avoid notifying the same one multiple times. */
         u1 = manager_get_unit_by_pid_cgroup(m, ucred->pid);
         if (u1) {
-                manager_invoke_notify_message(m, u1, ucred->pid, buf, fds);
+                manager_invoke_notify_message(m, u1, ucred, buf, fds);
                 found = true;
         }
 
         u2 = hashmap_get(m->watch_pids1, PID_TO_PTR(ucred->pid));
         if (u2 && u2 != u1) {
-                manager_invoke_notify_message(m, u2, ucred->pid, buf, fds);
+                manager_invoke_notify_message(m, u2, ucred, buf, fds);
                 found = true;
         }
 
         u3 = hashmap_get(m->watch_pids2, PID_TO_PTR(ucred->pid));
         if (u3 && u3 != u2 && u3 != u1) {
-                manager_invoke_notify_message(m, u3, ucred->pid, buf, fds);
+                manager_invoke_notify_message(m, u3, ucred, buf, fds);
                 found = true;
         }
 
diff --git a/src/core/service.c b/src/core/service.c
index d40007249b..66d3e45482 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -45,6 +45,8 @@
 #include "service.h"
 #include "signal-util.h"
 #include "special.h"
+#include "stat-util.h"
+#include "stdio-util.h"
 #include "string-table.h"
 #include "string-util.h"
 #include "strv.h"
@@ -797,9 +799,46 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) {
                         prefix, s->n_fd_store);
 }
 
+static int service_is_suitable_main_pid(Service *s, pid_t pid, int prio) {
+        Unit *owner;
+
+        assert(s);
+        assert(pid > 0);
+
+        /* Checks whether the specified PID is suitable as main PID for this service. returns negative if not, 0 if the
+         * PID is questionnable but should be accepted if the source of configuration is trusted. > 0 if the PID is
+         * good */
+
+        if (pid == getpid() || pid == 1) {
+                log_unit_full(UNIT(s), prio, 0, "New main PID "PID_FMT" is the manager, refusing.", pid);
+                return -EPERM;
+        }
+
+        if (pid == s->control_pid) {
+                log_unit_full(UNIT(s), prio, 0, "New main PID "PID_FMT" is the control process, refusing.", pid);
+                return -EPERM;
+        }
+
+        if (!pid_is_alive(pid)) {
+                log_unit_full(UNIT(s), prio, 0, "New main PID "PID_FMT" does not exist or is a zombie.", pid);
+                return -ESRCH;
+        }
+
+        owner = manager_get_unit_by_pid(UNIT(s)->manager, pid);
+        if (owner == UNIT(s)) {
+                log_unit_debug(UNIT(s), "New main PID "PID_FMT" belongs to service, we are happy.", pid);
+                return 1; /* Yay, it's definitely a good PID */
+        }
+
+        return 0; /* Hmm it's a suspicious PID, let's accept it if configuration source is trusted */
+}
+
+
 static int service_load_pid_file(Service *s, bool may_warn) {
+        char procfs[sizeof("/proc/self/fd/") - 1 + DECIMAL_STR_MAX(int)];
         _cleanup_free_ char *k = NULL;
-        int r;
+        _cleanup_close_ int fd = -1;
+        int r, prio;
         pid_t pid;
 
         assert(s);
@@ -807,30 +846,47 @@ static int service_load_pid_file(Service *s, bool may_warn) {
         if (!s->pid_file)
                 return -ENOENT;
 
-        r = read_one_line_file(s->pid_file, &k);
-        if (r < 0) {
-                if (may_warn)
-                        log_unit_info_errno(UNIT(s), r, "PID file %s not readable (yet?) after %s: %m", s->pid_file, service_state_to_string(s->state));
-                return r;
-        }
+        prio = may_warn ? LOG_INFO : LOG_DEBUG;
+
+        fd = chase_symlinks(s->pid_file, NULL, CHASE_OPEN|CHASE_SAFE, NULL);
+        if (fd == -EPERM)
+                return log_unit_full(UNIT(s), prio, 0, "Permission denied while opening PID file or unsafe symlink chain: %s", s->pid_file);
+        if (fd < 0)
+                return log_unit_full(UNIT(s), prio, 0, "Can't open PID file %s (yet?) after %s: %m", s->pid_file, service_state_to_string(s->state));
+
+        /* Let's read the PID file now that we chased it down. But we need to convert the O_PATH fd chase_symlinks() returned us into a proper fd first. */
+        xsprintf(procfs, "/proc/self/fd/%i", fd);
+        r = read_one_line_file(procfs, &k);
+        if (r < 0)
+                return log_unit_error_errno(UNIT(s), r, "Can't convert PID files %s O_PATH file descriptor to proper file descriptor: %m", s->pid_file);
 
         r = parse_pid(k, &pid);
-        if (r < 0) {
-                if (may_warn)
-                        log_unit_info_errno(UNIT(s), r, "Failed to read PID from file %s: %m", s->pid_file);
+        if (r < 0)
+                return log_unit_full(UNIT(s), prio, 0, "Failed to parse PID from file %s: %m", s->pid_file);
+
+        if (s->main_pid_known && pid == s->main_pid)
+                return 0;
+
+        r = service_is_suitable_main_pid(s, pid, prio);
+        if (r < 0)
                 return r;
-        }
+        if (r == 0) {
+                struct stat st;
 
-        if (!pid_is_alive(pid)) {
-                if (may_warn)
-                        log_unit_info(UNIT(s), "PID "PID_FMT" read from file %s does not exist or is a zombie.", pid, s->pid_file);
-                return -ESRCH;
+                /* Hmm, it's not clear if the new main PID is safe. Let's allow this if the PID file is owned by root */
+
+                if (fstat(fd, &st) < 0)
+                        return log_unit_error_errno(UNIT(s), errno, "Failed to fstat() PID file O_PATH fd: %m");
+
+                if (st.st_uid != 0) {
+                        log_unit_error(UNIT(s), "New main PID "PID_FMT" does not belong to service, and PID file is not owned by root. Refusing.", pid);
+                        return -EPERM;
+                }
+
+                log_unit_debug(UNIT(s), "New main PID "PID_FMT" does not belong to service, but we'll accept it since PID file is owned by root.", pid);
         }
 
         if (s->main_pid_known) {
-                if (pid == s->main_pid)
-                        return 0;
-
                 log_unit_debug(UNIT(s), "Main PID changing: "PID_FMT" -> "PID_FMT, s->main_pid, pid);
 
                 service_unwatch_main_pid(s);
@@ -849,7 +905,7 @@ static int service_load_pid_file(Service *s, bool may_warn) {
                 return r;
         }
 
-        return 0;
+        return 1;
 }
 
 static void service_search_main_pid(Service *s) {
@@ -2593,7 +2649,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                 /* Forking services may occasionally move to a new PID.
                  * As long as they update the PID file before exiting the old
                  * PID, they're fine. */
-                if (service_load_pid_file(s, false) == 0)
+                if (service_load_pid_file(s, false) > 0)
                         return;
 
                 s->main_pid = 0;
@@ -2971,37 +3027,74 @@ static int service_dispatch_watchdog(sd_event_source *source, usec_t usec, void
         return 0;
 }
 
-static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) {
+static bool service_notify_message_authorized(Service *s, pid_t pid, char **tags, FDSet *fds) {
+        assert(s);
+
+        if (s->notify_access == NOTIFY_NONE) {
+                log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception is disabled.", pid);
+                return false;
+        }
+
+        if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) {
+                if (s->main_pid != 0)
+                        log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid);
+                else
+                        log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid);
+
+                return false;
+        }
+
+        return true;
+}
+
+static void service_notify_message(
+                Unit *u,
+                const struct ucred *ucred,
+                char **tags,
+                FDSet *fds) {
+
         Service *s = SERVICE(u);
-        _cleanup_free_ char *cc = NULL;
         bool notify_dbus = false;
         const char *e;
+        int r;
 
         assert(u);
+        assert(ucred);
 
-        cc = strv_join(tags, ", ");
+        if (!service_notify_message_authorized(SERVICE(u), ucred->pid, tags, fds))
+                return;
 
         if (s->notify_access == NOTIFY_NONE) {
-                log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception is disabled.", pid);
-                return;
-        } else if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) {
-                if (s->main_pid != 0)
-                        log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid);
-                else
-                        log_unit_debug(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid);
-                return;
-        } else
-                log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", pid, isempty(cc) ? "n/a" : cc);
+                _cleanup_free_ char *cc = NULL;
+
+                cc = strv_join(tags, ", ");
+                log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", ucred->pid, isempty(cc) ? "n/a" : cc);
+        }
 
         /* Interpret MAINPID= */
         e = strv_find_startswith(tags, "MAINPID=");
         if (e && IN_SET(s->state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) {
-                if (parse_pid(e, &pid) < 0)
-                        log_unit_warning(u, "Failed to parse MAINPID= field in notification message: %s", e);
-                else {
-                        service_set_main_pid(s, pid);
-                        unit_watch_pid(UNIT(s), pid);
-                        notify_dbus = true;
+                pid_t new_main_pid;
+
+                if (parse_pid(e, &new_main_pid) < 0)
+                        log_unit_warning(u, "Failed to parse MAINPID= field in notification message, ignoring: %s", e);
+                else if (!s->main_pid_known || new_main_pid != s->main_pid) {
+
+                        r = service_is_suitable_main_pid(s, new_main_pid, LOG_WARNING);
+                        if (r == 0) {
+                                /* The new main PID is a bit suspicous, which is OK if the sender is privileged. */
+
+                                if (ucred->uid == 0) {
+                                        log_unit_debug(u, "New main PID "PID_FMT" does not belong to service, but we'll accept it as the request to change it came from a privileged process.", new_main_pid);
+                                        r = 1;
+                                } else
+                                        log_unit_debug(u, "New main PID "PID_FMT" does not belong to service, refusing.", new_main_pid);
+                        }
+                        if (r > 0) {
+                                service_set_main_pid(s, new_main_pid);
+                                unit_watch_pid(UNIT(s), new_main_pid);
+                                notify_dbus = true;
+                        }
                 }
         }
 
diff --git a/src/core/unit.h b/src/core/unit.h
index 1eabfa51e2..0f82cf3548 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -367,7 +367,7 @@ struct UnitVTable {
         void (*notify_cgroup_empty)(Unit *u);
 
         /* Called whenever a process of this unit sends us a message */
-        void (*notify_message)(Unit *u, pid_t pid, char **tags, FDSet *fds);
+        void (*notify_message)(Unit *u, const struct ucred *ucred, char **tags, FDSet *fds);
 
         /* Called whenever a name this Unit registered for comes or
          * goes away. */
diff --git a/test/TEST-20-MAINPIDGAMES/Makefile b/test/TEST-20-MAINPIDGAMES/Makefile
new file mode 100644
index 0000000000..e28368cf0a
--- /dev/null
+++ b/test/TEST-20-MAINPIDGAMES/Makefile
@@ -0,0 +1 @@
+../TEST-01-BASIC/Makefile
diff --git a/test/TEST-20-MAINPIDGAMES/test.sh b/test/TEST-20-MAINPIDGAMES/test.sh
new file mode 100644
index 0000000000..d9e20b7e25
--- /dev/null
+++ b/test/TEST-20-MAINPIDGAMES/test.sh
@@ -0,0 +1,82 @@
+#!/bin/bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+TEST_DESCRIPTION="test changing main PID"
+
+. $TEST_BASE_DIR/test-functions
+
+check_result_qemu() {
+    ret=1
+    mkdir -p $TESTDIR/root
+    mount ${LOOPDEV}p1 $TESTDIR/root
+    [[ -e $TESTDIR/root/testok ]] && ret=0
+    [[ -f $TESTDIR/root/failed ]] && cp -a $TESTDIR/root/failed $TESTDIR
+    [[ -f $TESTDIR/root/var/log/journal ]] && cp -a $TESTDIR/root/var/log/journal $TESTDIR
+    umount $TESTDIR/root
+    [[ -f $TESTDIR/failed ]] && cat $TESTDIR/failed
+    ls -l $TESTDIR/journal/*/*.journal
+    test -s $TESTDIR/failed && ret=$(($ret+1))
+    return $ret
+}
+
+test_run() {
+    if run_qemu; then
+        check_result_qemu || return 1
+    else
+        dwarn "can't run QEMU, skipping"
+    fi
+    if check_nspawn; then
+        run_nspawn
+        check_result_nspawn || return 1
+    else
+        dwarn "can't run systemd-nspawn, skipping"
+    fi
+    return 0
+}
+
+test_setup() {
+    create_empty_image
+    mkdir -p $TESTDIR/root
+    mount ${LOOPDEV}p1 $TESTDIR/root
+
+    (
+        LOG_LEVEL=5
+        eval $(udevadm info --export --query=env --name=${LOOPDEV}p2)
+
+        setup_basic_environment
+        inst_binary cut
+        inst_binary useradd
+        inst /etc/login.defs
+
+        # setup the testsuite service
+        cat >$initdir/etc/systemd/system/testsuite.service <<EOF
+[Unit]
+Description=Testsuite service
+
+[Service]
+ExecStart=/bin/bash -x /testsuite.sh
+Type=oneshot
+StandardOutput=tty
+StandardError=tty
+NotifyAccess=all
+EOF
+        cp testsuite.sh $initdir/
+
+        useradd -R $initdir -U -u 1234 test
+
+        setup_testsuite
+    )
+    setup_nspawn_root
+
+    ddebug "umount $TESTDIR/root"
+    umount $TESTDIR/root
+}
+
+test_cleanup() {
+    umount $TESTDIR/root 2>/dev/null
+    [[ $LOOPDEV ]] && losetup -d $LOOPDEV
+    return 0
+}
+
+do_test "$@"
+
diff --git a/test/TEST-20-MAINPIDGAMES/testsuite.sh b/test/TEST-20-MAINPIDGAMES/testsuite.sh
new file mode 100644
index 0000000000..fe97a32018
--- /dev/null
+++ b/test/TEST-20-MAINPIDGAMES/testsuite.sh
@@ -0,0 +1,190 @@
+
+#!/bin/bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+set -ex
+set -o pipefail
+
+systemctl_show_value() {
+    systemctl show "$@" | cut -d = -f 2-
+}
+
+systemd-analyze set-log-level debug
+
+test `systemctl_show_value -p MainPID testsuite.service` -eq $$
+
+# Start a test process inside of our own cgroup
+sleep infinity &
+INTERNALPID=$!
+disown
+
+# Start a test process outside of our own cgroup
+systemd-run -p User=test --unit=sleep.service /bin/sleep infinity
+EXTERNALPID=`systemctl_show_value -p MainPID sleep.service`
+
+# Update our own main PID to the external test PID, this should work
+systemd-notify MAINPID=$EXTERNALPID
+test `systemctl_show_value -p MainPID testsuite.service` -eq $EXTERNALPID
+
+# Update our own main PID to the internal test PID, this should work, too
+systemd-notify MAINPID=$INTERNALPID
+test `systemctl_show_value -p MainPID testsuite.service` -eq $INTERNALPID
+
+# Update it back to our own PID, this should also work
+systemd-notify MAINPID=$$
+test `systemctl_show_value -p MainPID testsuite.service` -eq $$
+
+# Try to set it to PID 1, which it should ignore, because that's the manager
+systemd-notify MAINPID=1
+test `systemctl_show_value -p MainPID testsuite.service` -eq $$
+
+# Try to set it to PID 0, which is invalid and should be ignored
+systemd-notify MAINPID=0
+test `systemctl_show_value -p MainPID testsuite.service` -eq $$
+
+# Try to set it to a valid but non-existing PID, which should be ignored. (Note
+# that we set the PID to a value well above any known /proc/sys/kernel/pid_max,
+# which means we can be pretty sure it doesn't exist by coincidence)
+systemd-notify MAINPID=1073741824
+test `systemctl_show_value -p MainPID testsuite.service` -eq $$
+
+# Change it again to the external PID, without priviliges this time. This should be ignored, because the PID is from outside of our cgroup and we lack privileges.
+systemd-notify --uid=1000 MAINPID=$EXTERNALPID
+test `systemctl_show_value -p MainPID testsuite.service` -eq $$
+
+# Change it again to the internal PID, without priviliges this time. This should work, as the process is on our cgroup, and that's enough even if we lack privileges.
+systemd-notify --uid=1000 MAINPID=$INTERNALPID
+test `systemctl_show_value -p MainPID testsuite.service` -eq $INTERNALPID
+
+# Update it back to our own PID, this should also work
+systemd-notify --uid=1000 MAINPID=$$
+test `systemctl_show_value -p MainPID testsuite.service` -eq $$
+
+cat >/tmp/mainpid.sh <<EOF
+#!/bin/bash
+
+set -eux
+set -o pipefail
+
+# Create a number of children, and make one the main one
+sleep infinity &
+disown
+
+sleep infinity &
+MAINPID=\$!
+disown
+
+sleep infinity &
+disown
+
+echo \$MAINPID > /run/mainpidsh/pid
+EOF
+chmod +x /tmp/mainpid.sh
+
+cat > /etc/systemd/system/mainpidsh.service <<EOF
+[Unit]
+Description=MainPID test 1 service
+
+[Service]
+StandardOutput=tty
+StandardError=tty
+Type=forking
+RuntimeDirectory=mainpidsh
+PIDFile=/run/mainpidsh/pid
+ExecStart=/tmp/mainpid.sh
+EOF
+
+systemctl daemon-reload
+systemctl start mainpidsh.service
+test `systemctl_show_value -p MainPID mainpidsh.service` -eq `cat /run/mainpidsh/pid`
+
+cat >/tmp/mainpid2.sh <<EOF
+#!/bin/bash
+
+set -eux
+set -o pipefail
+
+# Create a number of children, and make one the main one
+sleep infinity &
+disown
+
+sleep infinity &
+MAINPID=\$!
+disown
+
+sleep infinity &
+disown
+
+echo \$MAINPID > /run/mainpidsh2/pid
+chown 1001:1001 /run/mainpidsh2/pid
+EOF
+chmod +x /tmp/mainpid2.sh
+
+cat > /etc/systemd/system/mainpidsh2.service <<EOF
+[Unit]
+Description=MainPID test 2 service
+
+[Service]
+StandardOutput=tty
+StandardError=tty
+Type=forking
+RuntimeDirectory=mainpidsh2
+PIDFile=/run/mainpidsh2/pid
+ExecStart=/tmp/mainpid2.sh
+EOF
+
+systemctl daemon-reload
+systemctl start mainpidsh2.service
+test `systemctl_show_value -p MainPID mainpidsh2.service` -eq `cat /run/mainpidsh2/pid`
+
+cat >/dev/shm/mainpid3.sh <<EOF
+#!/bin/bash
+
+set -eux
+set -o pipefail
+
+sleep infinity &
+disown
+
+sleep infinity &
+disown
+
+sleep infinity &
+disown
+
+# Let's try to play games, and link up a privileged PID file
+ln -s ../mainpidsh/pid /run/mainpidsh3/pid
+
+# Quick assertion that the link isn't dead
+test -f /run/mainpidsh3/pid
+EOF
+chmod 755 /dev/shm/mainpid3.sh
+
+cat > /etc/systemd/system/mainpidsh3.service <<EOF
+[Unit]
+Description=MainPID test 3 service
+
+[Service]
+StandardOutput=tty
+StandardError=tty
+Type=forking
+RuntimeDirectory=mainpidsh3
+PIDFile=/run/mainpidsh3/pid
+User=test
+TimeoutStartSec=2s
+ExecStart=/dev/shm/mainpid3.sh
+EOF
+
+systemctl daemon-reload
+systemctl start mainpidsh3.service
+
+# Test that this failed due to timeout, and not some other error
+# test `systemctl_show_value -p Result mainpidsh3.service` = timeout
+# Just check that there is no MainPID => the pid file was ignored
+test `systemctl_show_value -p MainPID mainpidsh3.service` -eq 0
+
+systemd-analyze set-log-level info
+
+echo OK > /testok
+
+exit 0
diff --git a/test/test-functions b/test/test-functions
index 567a000b8d..9fd5c7ec0f 100644
--- a/test/test-functions
+++ b/test/test-functions
@@ -18,7 +18,7 @@ if ! ROOTLIBDIR=$(pkg-config --variable=systemdutildir systemd); then
     ROOTLIBDIR=/usr/lib/systemd
 fi
 
-BASICTOOLS="sh bash setsid loadkeys setfont login sulogin gzip sleep echo mount umount cryptsetup date dmsetup modprobe sed cmp tee rm"
+BASICTOOLS="test sh bash setsid loadkeys setfont login sulogin gzip sleep echo mount umount cryptsetup date dmsetup modprobe sed cmp tee rm chmod chown ln"
 DEBUGTOOLS="df free ls stty cat ps ln ip route dmesg dhclient mkdir cp ping dhclient strace less grep id tty touch du sort hostname find"
 
 function find_qemu_bin() {