test: fix fd_is_mount_point() check

So the currentl and only fd_is_mount_point() check is actually entirely
bogus: it passes "/" as filename argument, but that's not actually a
a valid filename, but an absolute path.

fd_is_mount_point() is written in a way tha the fd refers to a directory
and the specified path is a file directly below it that shall be
checked. The test call actually violated that rule, but still expected
success.

Let's fix this, and check for this explicitly, and refuse it.

Let's extend the test and move it to test-mountpoint-util.c where the
rest of the tests for related calls are placed.

Replaces: #18004
Fixes: #17950

README

@@ -187,7 +187,7 @@ REQUIREMENTS:
         polkit (optional)
 
         To build in directory build/:
-          meson build/ && ninja -C build
+          meson setup build/ && meson compile -C build/
 
         Any configuration options can be specified as -Darg=value... arguments
         to meson. After the build directory is initially configured, meson will
@@ -197,10 +197,10 @@ REQUIREMENTS:
         their current values.
 
         Useful commands:
-          ninja -v some/target
+          meson compile -v -C build/ some/target
-          ninja test
+          meson test -C build/
-          sudo ninja install
+          sudo meson install -C build/
-          DESTDIR=... ninja install
+          DESTDIR=... meson install -C build/
 
         A tarball can be created with:
           git archive --format=tar --prefix=systemd-222/ v222 | xz > systemd-222.tar.xz

docs/CODE_QUALITY.md

@@ -10,13 +10,15 @@ The systemd project has a number of code quality tools set up in the source
 tree and on the github infrastructure. Here's an incomprehensive list of the
 available functionality:
 
-1. Use `ninja -C build test` to run the unit tests. Some tests are skipped if
+1. Use `meson test -C build` to run the unit tests. Some tests are skipped if
    no privileges are available, hence consider also running them with `sudo
-   ninja -C build test`. A couple of unit tests are considered "unsafe" (as
+   meson test -C build`. A couple of unit tests are considered "unsafe" (as
-   they change system state); to run those too, build with `meson
+   they change system state); to run those too, build with `meson setup
    -Dtests=unsafe`. Finally, some unit tests are considered to be very slow,
-   build them too with `meson -Dslow-tests=true`. (Note that there are a couple
+   build them too with `meson setup -Dslow-tests=true`. (Note that there are a
-   of manual tests in addition to these unit tests.)
+   couple of manual tests in addition to these unit tests.) (Also note: you can
+   change these flags for an already set up build tree, too, with "meson
+   configure -C build -D…".)
 
 2. Use `./test/run-integration-tests.sh` to run the full integration test
    suite. This will build OS images with a number of integration tests and run
@@ -35,14 +37,14 @@ available functionality:
    `./tools/find-tabs.sh recpatch` to fix them. (Again, grain of salt, foreign
    headers should usually be left unmodified.)
 
-6. Use `ninja -C build check-api-docs` to compare the list of exported
+6. Use `meson compile -C build check-api-docs` to compare the list of exported
    symbols of `libsystemd.so` and `libudev.so` with the list of man pages. Symbols
    lacking documentation are highlighted.
 
-7. Use `ninja -C build hwdb-update` to automatically download and import the
+7. Use `meson compile -C build hwdb-update` to automatically download and import the
    PCI, USB and OUI databases into hwdb.
 
-8. Use `ninja -C build man/update-man-rules` to update the meson rules for
+8. Use `meson compile -C build man/update-man-rules` to update the meson rules for
    building man pages automatically from the docbook XML files included in
    `man/`.
 

docs/HACKING.md

@@ -81,8 +81,8 @@ $ git clone https://github.com/systemd/systemd.git
 $ cd systemd
 $ vim src/core/main.c                     # or wherever you'd like to make your changes
 $ meson build                             # configure the build
-$ ninja -C build                          # build it locally, see if everything compiles fine
+$ meson compile -C build                  # build it locally, see if everything compiles fine
-$ ninja -C build test                     # run some simple regression tests
+$ meson test -C build                     # run some simple regression tests
 $ ln -s .mkosi/mkosi.fedora mkosi.default # Configure mkosi to build a fedora image
 $ (umask 077; echo 123 > mkosi.rootpw)    # set root password used by mkosi
 $ sudo mkosi                              # build a test image

docs/RELEASE.md

@@ -7,14 +7,14 @@ layout: default
 # Steps to a Successful Release
 
 1. Add all items to NEWS
-2. Update the contributors list in NEWS (`ninja -C build git-contrib`)
+2. Update the contributors list in NEWS (`meson compile -C build git-contrib`)
 3. Update the time and place in NEWS
 4. [RC1] Update version and library numbers in `meson.build`
-5. Check dbus docs with `ninja -C build man/update-dbus-docs`
+5. Check dbus docs with `meson compile -C build man/update-dbus-docs`
 6. Tag the release: `version=vXXX-rcY && git tag -s "${version}" -m "systemd ${version}"`
-7. Do `ninja -C build`
+7. Do `meson compile -C build`
 8. Make sure that the version string and package string match: `build/systemctl --version`
-9. Upload the documentation: `ninja -C build doc-sync`
+9. Upload the documentation: `meson compile -C build doc-sync`
 10. [FINAL] Close the github milestone and open a new one (https://github.com/systemd/systemd/milestones)
 11. "Draft" a new release on github (https://github.com/systemd/systemd/releases/new), mark "This is a pre-release" if appropriate.
 12. Check that announcement to systemd-devel, with a copy&paste from NEWS, was sent. This should happen automatically.

docs/TRANSLATORS.md

@@ -26,7 +26,7 @@ To create a translation to a language not yet available, start by creating the
 initial template:
 
 ```
-$ ninja -C build/ systemd-pot
+$ meson compile -C build/ systemd-pot
 ```
 
 This will generate file `po/systemd.pot` in the source tree.
@@ -50,7 +50,7 @@ using the `poedit` GUI editor.)
 Start by updating the `*.po` files from the latest template:
 
 ```
-$ ninja -C build/ systemd-update-po
+$ meson compile -C build/ systemd-update-po
 ```
 
 This will touch all the `*.po` files, so you'll want to pay attention when
@@ -74,7 +74,7 @@ using `git checkout -- po/` after you commit the changes you do want to keep.)
 You can recompile the `*.po` files using the following command:
 
 ```
-$ ninja -C build/ systemd-gmo
+$ meson compile -C build/ systemd-gmo
 ```
 
 The resulting files will be saved in the `build/po/` directory.

src/basic/mountpoint-util.c

@@ -132,6 +132,29 @@ static int fd_fdinfo_mnt_id(int fd, const char *filename, int flags, int *ret_mn
         return safe_atoi(p, ret_mnt_id);
 }
 
+static bool filename_possibly_with_slash_suffix(const char *s) {
+        const char *slash, *copied;
+
+        /* Checks whether the specified string is either file name, or a filename with a suffix of
+         * slashes. But nothing else.
+         *
+         * this is OK: foo, bar, foo/, bar/, foo//, bar///
+         * this is not OK: "", "/", "/foo", "foo/bar", ".", ".." … */
+
+        slash = strchr(s, '/');
+        if (!slash)
+                return filename_is_valid(s);
+
+        if (slash - s > FILENAME_MAX) /* We want to allocate on the stack below, hence do a size check first */
+                return false;
+
+        if (slash[strspn(slash, "/")] != 0) /* Check that the suffix consist only of one or more slashes */
+                return false;
+
+        copied = strndupa(s, slash - s);
+        return filename_is_valid(copied);
+}
+
 int fd_is_mount_point(int fd, const char *filename, int flags) {
         _cleanup_free_ struct file_handle *h = NULL, *h_parent = NULL;
         int mount_id = -1, mount_id_parent = -1;
@@ -144,6 +167,11 @@ int fd_is_mount_point(int fd, const char *filename, int flags) {
         assert(filename);
         assert((flags & ~(AT_SYMLINK_FOLLOW|AT_EMPTY_PATH)) == 0);
 
+        /* Insist that the specified filename is actually a filename, and not a path, i.e. some inode further
+         * up or down the tree then immediately below the specified directory fd. */
+        if (!filename_possibly_with_slash_suffix(filename))
+                return -EINVAL;
+
         /* First we will try statx()' STATX_ATTR_MOUNT_ROOT attribute, which is our ideal API, available
          * since kernel 5.8.
          *

src/test/test-mountpoint-util.c

@@ -256,6 +256,37 @@ static void test_path_is_mount_point(void) {
         assert_se(rm_rf(tmp_dir, REMOVE_ROOT|REMOVE_PHYSICAL) == 0);
 }
 
+static void test_fd_is_mount_point(void) {
+        _cleanup_close_ int fd = -1;
+
+        log_info("/* %s */", __func__);
+
+        fd = open("/", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY);
+        assert_se(fd >= 0);
+
+        /* Not allowed, since "/" is a path, not a plain filename */
+        assert_se(fd_is_mount_point(fd, "/", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, ".", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "./", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "..", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "../", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "/proc", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "/proc/", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "proc/sys", 0) == -EINVAL);
+        assert_se(fd_is_mount_point(fd, "proc/sys/", 0) == -EINVAL);
+
+        /* This one definitely is a mount point */
+        assert_se(fd_is_mount_point(fd, "proc", 0) > 0);
+        assert_se(fd_is_mount_point(fd, "proc/", 0) > 0);
+
+        /* /root's entire raison d'etre is to be on the root file system (i.e. not in /home/ which might be
+         * split off), so that the user can always log in, so it cannot be a mount point unless the system is
+         * borked. Let's allow for it to be missing though. */
+        assert_se(IN_SET(fd_is_mount_point(fd, "root", 0), -ENOENT, 0));
+        assert_se(IN_SET(fd_is_mount_point(fd, "root/", 0), -ENOENT, 0));
+}
+
 int main(int argc, char *argv[]) {
         test_setup_logging(LOG_DEBUG);
 
@@ -279,6 +310,7 @@ int main(int argc, char *argv[]) {
 
         test_mnt_id();
         test_path_is_mount_point();
+        test_fd_is_mount_point();
 
         return 0;
 }

src/test/test-path-util.c

@@ -7,7 +7,6 @@
 #include "exec-util.h"
 #include "fd-util.h"
 #include "macro.h"
-#include "mountpoint-util.h"
 #include "path-util.h"
 #include "process-util.h"
 #include "rm-rf.h"
@@ -42,8 +41,6 @@ static void test_path_simplify(const char *in, const char *out, const char *out_
 }
 
 static void test_path(void) {
-        _cleanup_close_ int fd = -1;
-
         log_info("/* %s */", __func__);
 
         test_path_compare("/goo", "/goo", 0);
@@ -82,10 +79,6 @@ static void test_path(void) {
         assert_se(streq(basename("/aa///file..."), "file..."));
         assert_se(streq(basename("file.../"), ""));
 
-        fd = open("/", O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY);
-        assert_se(fd >= 0);
-        assert_se(fd_is_mount_point(fd, "/", 0) > 0);
-
         test_path_simplify("aaa/bbb////ccc", "aaa/bbb/ccc", "aaa/bbb/ccc");
         test_path_simplify("//aaa/.////ccc", "/aaa/./ccc", "/aaa/ccc");
         test_path_simplify("///", "/", "/");