boot: suppress XBOOTLDR if same device as ESP when enumerating entries

On my local system I linked up the ESP and XBOOTLDR partitions, and
ended up with duplicate entries being listed. Try hard to detect that
and only enumerate entries in the ESP if it turns out that both dirs
have the same dev_t.

This should detect both bind mounted and symlinked cases and should make
our list output less confusing.

5 files changed, 115 insertions, 39 deletions

diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h
index f7d2f12aa9..f9b3d2fbed 100644
--- a/src/basic/stat-util.h
+++ b/src/basic/stat-util.h
@@ -114,3 +114,9 @@ int statx_fallback(int dfd, const char *path, int flags, unsigned mask, struct s
                 struct new_statx nsx;           \
         } var
 #endif
+
+static inline bool devid_set_and_equal(dev_t a, dev_t b) {
+        /* Returns true if a and b definitely refer to the same device. If either is zero, this means "don't
+         * know" and we'll return false */
+        return a == b && a != 0;
+}
diff --git a/src/boot/bless-boot.c b/src/boot/bless-boot.c
index 9e4b0d1f72..2cbabc6c59 100644
--- a/src/boot/bless-boot.c
+++ b/src/boot/bless-boot.c
@@ -14,6 +14,7 @@
 #include "parse-util.h"
 #include "path-util.h"
 #include "pretty-print.h"
+#include "stat-util.h"
 #include "sync-util.h"
 #include "terminal-util.h"
 #include "util.h"
@@ -98,17 +99,18 @@ static int parse_argv(int argc, char *argv[]) {
 
 static int acquire_path(void) {
         _cleanup_free_ char *esp_path = NULL, *xbootldr_path = NULL;
+        dev_t esp_devid = 0, xbootldr_devid = 0;
         char **a;
         int r;
 
         if (!strv_isempty(arg_path))
                 return 0;
 
-        r = find_esp_and_warn(NULL, false, &esp_path, NULL, NULL, NULL, NULL);
+        r = find_esp_and_warn(NULL, /* unprivileged_mode= */ false, &esp_path, NULL, NULL, NULL, NULL, &esp_devid);
         if (r < 0 && r != -ENOKEY) /* ENOKEY means not found, and is the only error the function won't log about on its own */
                 return r;
 
-        r = find_xbootldr_and_warn(NULL, false, &xbootldr_path, NULL);
+        r = find_xbootldr_and_warn(NULL, /* unprivileged_mode= */ false, &xbootldr_path, NULL, &xbootldr_devid);
         if (r < 0 && r != -ENOKEY)
                 return r;
 
@@ -117,8 +119,10 @@ static int acquire_path(void) {
                                        "Couldn't find $BOOT partition. It is recommended to mount it to /boot.\n"
                                        "Alternatively, use --path= to specify path to mount point.");
 
-        if (esp_path)
+        if (esp_path && xbootldr_path && !devid_set_and_equal(esp_devid, xbootldr_devid)) /* in case the two paths refer to the same inode, suppress one */
                 a = strv_new(esp_path, xbootldr_path);
+        else if (esp_path)
+                a = strv_new(esp_path);
         else
                 a = strv_new(xbootldr_path);
         if (!a)
@@ -130,7 +134,7 @@ static int acquire_path(void) {
                 _cleanup_free_ char *j = NULL;
 
                 j = strv_join(arg_path, ":");
-                log_debug("Using %s as boot loader drop-in search path.", j);
+                log_debug("Using %s as boot loader drop-in search path.", strna(j));
         }
 
         return 0;
diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c
index ab514d28ee..c81411c857 100644
--- a/src/boot/bootctl.c
+++ b/src/boot/bootctl.c
@@ -75,7 +75,8 @@ static int acquire_esp(
                 uint32_t *ret_part,
                 uint64_t *ret_pstart,
                 uint64_t *ret_psize,
-                sd_id128_t *ret_uuid) {
+                sd_id128_t *ret_uuid,
+                dev_t *ret_devid) {
 
         char *np;
         int r;
@@ -86,7 +87,7 @@ static int acquire_esp(
          * we simply eat up the error here, so that --list and --status work too, without noise about
          * this). */
 
-        r = find_esp_and_warn(arg_esp_path, unprivileged_mode, &np, ret_part, ret_pstart, ret_psize, ret_uuid);
+        r = find_esp_and_warn(arg_esp_path, unprivileged_mode, &np, ret_part, ret_pstart, ret_psize, ret_uuid, ret_devid);
         if (r == -ENOKEY) {
                 if (graceful)
                         return log_info_errno(r, "Couldn't find EFI system partition, skipping.");
@@ -104,16 +105,23 @@ static int acquire_esp(
         return 1;
 }
 
-static int acquire_xbootldr(bool unprivileged_mode, sd_id128_t *ret_uuid) {
+static int acquire_xbootldr(
+                bool unprivileged_mode,
+                sd_id128_t *ret_uuid,
+                dev_t *ret_devid) {
+
         char *np;
         int r;
 
-        r = find_xbootldr_and_warn(arg_xbootldr_path, unprivileged_mode, &np, ret_uuid);
+        r = find_xbootldr_and_warn(arg_xbootldr_path, unprivileged_mode, &np, ret_uuid, ret_devid);
         if (r == -ENOKEY) {
                 log_debug_errno(r, "Didn't find an XBOOTLDR partition, using the ESP as $BOOT.");
+                arg_xbootldr_path = mfree(arg_xbootldr_path);
+
                 if (ret_uuid)
                         *ret_uuid = SD_ID128_NULL;
-                arg_xbootldr_path = mfree(arg_xbootldr_path);
+                if (ret_devid)
+                        *ret_devid = 0;
                 return 0;
         }
         if (r < 0)
@@ -1436,7 +1444,7 @@ static void print_yes_no_line(bool first, bool good, const char *name) {
 static int are_we_installed(void) {
         int r;
 
-        r = acquire_esp(/* privileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, NULL);
+        r = acquire_esp(/* privileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, NULL, NULL);
         if (r < 0)
                 return r;
 
@@ -1468,9 +1476,10 @@ static int are_we_installed(void) {
 
 static int verb_status(int argc, char *argv[], void *userdata) {
         sd_id128_t esp_uuid = SD_ID128_NULL, xbootldr_uuid = SD_ID128_NULL;
+        dev_t esp_devid = 0, xbootldr_devid = 0;
         int r, k;
 
-        r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, &esp_uuid);
+        r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, &esp_uuid, &esp_devid);
         if (arg_print_esp_path) {
                 if (r == -EACCES) /* If we couldn't acquire the ESP path, log about access errors (which is the only
                                    * error the find_esp_and_warn() won't log on its own) */
@@ -1481,7 +1490,7 @@ static int verb_status(int argc, char *argv[], void *userdata) {
                 puts(arg_esp_path);
         }
 
-        r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid);
+        r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid, &xbootldr_devid);
         if (arg_print_dollar_boot_path) {
                 if (r == -EACCES)
                         return log_error_errno(r, "Failed to determine XBOOTLDR location: %m");
@@ -1615,7 +1624,14 @@ static int verb_status(int argc, char *argv[], void *userdata) {
         }
 
         if (arg_esp_path || arg_xbootldr_path) {
-                k = status_entries(arg_esp_path, esp_uuid, arg_xbootldr_path, xbootldr_uuid);
+                /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */
+                bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid);
+
+                k = status_entries(
+                                arg_esp_path,
+                                esp_uuid,
+                                same ? NULL : arg_xbootldr_path,
+                                same ? SD_ID128_NULL : xbootldr_uuid);
                 if (k < 0)
                         r = k;
         }
@@ -1626,25 +1642,29 @@ static int verb_status(int argc, char *argv[], void *userdata) {
 static int verb_list(int argc, char *argv[], void *userdata) {
         _cleanup_(boot_config_free) BootConfig config = {};
         _cleanup_strv_free_ char **efi_entries = NULL;
+        dev_t esp_devid = 0, xbootldr_devid = 0;
         int r;
 
         /* If we lack privileges we invoke find_esp_and_warn() in "unprivileged mode" here, which does two things: turn
          * off logging about access errors and turn off potentially privileged device probing. Here we're interested in
          * the latter but not the former, hence request the mode, and log about EACCES. */
 
-        r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL);
+        r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL, &esp_devid);
         if (r == -EACCES) /* We really need the ESP path for this call, hence also log about access errors */
                 return log_error_errno(r, "Failed to determine ESP: %m");
         if (r < 0)
                 return r;
 
-        r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, NULL);
+        r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, NULL, &xbootldr_devid);
         if (r == -EACCES)
                 return log_error_errno(r, "Failed to determine XBOOTLDR partition: %m");
         if (r < 0)
                 return r;
 
-        r = boot_entries_load_config(arg_esp_path, arg_xbootldr_path, &config);
+        /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */
+        bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid);
+
+        r = boot_entries_load_config(arg_esp_path, same ? NULL : arg_xbootldr_path, &config);
         if (r < 0)
                 return r;
 
@@ -1840,7 +1860,7 @@ static int verb_install(int argc, char *argv[], void *userdata) {
         install = streq(argv[0], "install");
         graceful = !install && arg_graceful; /* support graceful mode for updates */
 
-        r = acquire_esp(/* unprivileged_mode= */ false, graceful, &part, &pstart, &psize, &uuid);
+        r = acquire_esp(/* unprivileged_mode= */ false, graceful, &part, &pstart, &psize, &uuid, NULL);
         if (graceful && r == -ENOKEY)
                 return 0; /* If --graceful is specified and we can't find an ESP, handle this cleanly */
         if (r < 0)
@@ -1857,7 +1877,7 @@ static int verb_install(int argc, char *argv[], void *userdata) {
                 }
         }
 
-        r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL);
+        r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL, NULL);
         if (r < 0)
                 return r;
 
@@ -1917,11 +1937,11 @@ static int verb_remove(int argc, char *argv[], void *userdata) {
         sd_id128_t uuid = SD_ID128_NULL;
         int r, q;
 
-        r = acquire_esp(/* unprivileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, &uuid);
+        r = acquire_esp(/* unprivileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, &uuid, NULL);
         if (r < 0)
                 return r;
 
-        r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL);
+        r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL, NULL);
         if (r < 0)
                 return r;
 
@@ -2130,7 +2150,7 @@ static int verb_set_efivar(int argc, char *argv[], void *userdata) {
 static int verb_random_seed(int argc, char *argv[], void *userdata) {
         int r;
 
-        r = find_esp_and_warn(arg_esp_path, false, &arg_esp_path, NULL, NULL, NULL, NULL);
+        r = find_esp_and_warn(arg_esp_path, false, &arg_esp_path, NULL, NULL, NULL, NULL, NULL);
         if (r == -ENOKEY) {
                 /* find_esp_and_warn() doesn't warn about ENOKEY, so let's do that on our own */
                 if (!arg_graceful)
diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c
index 7c74cf6f18..1dbeda23d9 100644
--- a/src/shared/bootspec.c
+++ b/src/shared/bootspec.c
@@ -780,6 +780,7 @@ int boot_entries_load_config_auto(
                 BootConfig *config) {
 
         _cleanup_free_ char *esp_where = NULL, *xbootldr_where = NULL;
+        dev_t esp_devid = 0, xbootldr_devid = 0;
         int r;
 
         assert(config);
@@ -800,14 +801,18 @@ int boot_entries_load_config_auto(
                                                "Failed to determine whether /run/boot-loader-entries/ exists: %m");
         }
 
-        r = find_esp_and_warn(override_esp_path, false, &esp_where, NULL, NULL, NULL, NULL);
+        r = find_esp_and_warn(override_esp_path, /* unprivileged_mode= */ false, &esp_where, NULL, NULL, NULL, NULL, &esp_devid);
         if (r < 0) /* we don't log about ENOKEY here, but propagate it, leaving it to the caller to log */
                 return r;
 
-        r = find_xbootldr_and_warn(override_xbootldr_path, false, &xbootldr_where, NULL);
+        r = find_xbootldr_and_warn(override_xbootldr_path, /* unprivileged_mode= */ false, &xbootldr_where, NULL, &xbootldr_devid);
         if (r < 0 && r != -ENOKEY)
                 return r; /* It's fine if the XBOOTLDR partition doesn't exist, hence we ignore ENOKEY here */
 
+        /* If both paths actually refer to the same inode, suppress the xbootldr path */
+        if (esp_where && xbootldr_where && devid_set_and_equal(esp_devid, xbootldr_devid))
+                xbootldr_where = mfree(xbootldr_where);
+
         return boot_entries_load_config(esp_where, xbootldr_where, config);
 }
 
@@ -1162,7 +1167,8 @@ static int verify_esp(
                 uint32_t *ret_part,
                 uint64_t *ret_pstart,
                 uint64_t *ret_psize,
-                sd_id128_t *ret_uuid) {
+                sd_id128_t *ret_uuid,
+                dev_t *ret_devid) {
 
         bool relax_checks;
         dev_t devid;
@@ -1212,9 +1218,16 @@ static int verify_esp(
          * however blkid can't work if we have no privileges to access block devices directly, which is why
          * we use udev in that case. */
         if (unprivileged_mode)
-                return verify_esp_udev(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid);
+                r = verify_esp_udev(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid);
         else
-                return verify_esp_blkid(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid);
+                r = verify_esp_blkid(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid);
+        if (r < 0)
+                return r;
+
+        if (ret_devid)
+                *ret_devid = devid;
+
+        return 0;
 
 finish:
         if (ret_part)
@@ -1225,6 +1238,8 @@ finish:
                 *ret_psize = 0;
         if (ret_uuid)
                 *ret_uuid = SD_ID128_NULL;
+        if (ret_devid)
+                *ret_devid = 0;
 
         return 0;
 }
@@ -1236,7 +1251,8 @@ int find_esp_and_warn(
                 uint32_t *ret_part,
                 uint64_t *ret_pstart,
                 uint64_t *ret_psize,
-                sd_id128_t *ret_uuid) {
+                sd_id128_t *ret_uuid,
+                dev_t *ret_devid) {
 
         int r;
 
@@ -1247,7 +1263,7 @@ int find_esp_and_warn(
          */
 
         if (path) {
-                r = verify_esp(path, false, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid);
+                r = verify_esp(path, /* searching= */ false, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid, ret_devid);
                 if (r < 0)
                         return r;
 
@@ -1256,13 +1272,21 @@ int find_esp_and_warn(
 
         path = getenv("SYSTEMD_ESP_PATH");
         if (path) {
+                struct stat st;
+
                 if (!path_is_valid(path) || !path_is_absolute(path))
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                                "$SYSTEMD_ESP_PATH does not refer to absolute path, refusing to use it: %s",
                                                path);
 
-                /* Note: when the user explicitly configured things with an env var we won't validate the mount
-                 * point. After all we want this to be useful for testing. */
+                /* Note: when the user explicitly configured things with an env var we won't validate the
+                 * path beyond checking it refers to a directory. After all we want this to be useful for
+                 * testing. */
+
+                if (stat(path, &st) < 0)
+                        return log_error_errno(errno, "Failed to stat '%s': %m", path);
+                if (!S_ISDIR(st.st_mode))
+                        return log_error_errno(SYNTHETIC_ERRNO(ENOTDIR), "ESP path '%s' is not a directory.", path);
 
                 if (ret_part)
                         *ret_part = 0;
@@ -1272,13 +1296,15 @@ int find_esp_and_warn(
                         *ret_psize = 0;
                 if (ret_uuid)
                         *ret_uuid = SD_ID128_NULL;
+                if (ret_devid)
+                        *ret_devid = st.st_dev;
 
                 goto found;
         }
 
         FOREACH_STRING(path, "/efi", "/boot", "/boot/efi") {
 
-                r = verify_esp(path, true, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid);
+                r = verify_esp(path, /* searching= */ true, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid, ret_devid);
                 if (r >= 0)
                         goto found;
                 if (!IN_SET(r, -ENOENT, -EADDRNOTAVAIL)) /* This one is not it */
@@ -1445,7 +1471,8 @@ static int verify_xbootldr(
                 const char *p,
                 bool searching,
                 bool unprivileged_mode,
-                sd_id128_t *ret_uuid) {
+                sd_id128_t *ret_uuid,
+                dev_t *ret_devid) {
 
         bool relax_checks;
         dev_t devid;
@@ -1463,13 +1490,22 @@ static int verify_xbootldr(
                 goto finish;
 
         if (unprivileged_mode)
-                return verify_xbootldr_udev(devid, searching, ret_uuid);
+                r = verify_xbootldr_udev(devid, searching, ret_uuid);
         else
-                return verify_xbootldr_blkid(devid, searching, ret_uuid);
+                r = verify_xbootldr_blkid(devid, searching, ret_uuid);
+        if (r < 0)
+                return r;
+
+        if (ret_devid)
+                *ret_devid = devid;
+
+        return 0;
 
 finish:
         if (ret_uuid)
                 *ret_uuid = SD_ID128_NULL;
+        if (ret_devid)
+                *ret_devid = 0;
 
         return 0;
 }
@@ -1478,14 +1514,15 @@ int find_xbootldr_and_warn(
                 const char *path,
                 bool unprivileged_mode,
                 char **ret_path,
-                sd_id128_t *ret_uuid) {
+                sd_id128_t *ret_uuid,
+                dev_t *ret_devid) {
 
         int r;
 
         /* Similar to find_esp_and_warn(), but finds the XBOOTLDR partition. Returns the same errors. */
 
         if (path) {
-                r = verify_xbootldr(path, false, unprivileged_mode, ret_uuid);
+                r = verify_xbootldr(path, /* searching= */ false, unprivileged_mode, ret_uuid, ret_devid);
                 if (r < 0)
                         return r;
 
@@ -1494,18 +1531,27 @@ int find_xbootldr_and_warn(
 
         path = getenv("SYSTEMD_XBOOTLDR_PATH");
         if (path) {
+                struct stat st;
+
                 if (!path_is_valid(path) || !path_is_absolute(path))
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                                "$SYSTEMD_XBOOTLDR_PATH does not refer to absolute path, refusing to use it: %s",
                                                path);
 
+                if (stat(path, &st) < 0)
+                        return log_error_errno(errno, "Failed to stat '%s': %m", path);
+                if (!S_ISDIR(st.st_mode))
+                        return log_error_errno(SYNTHETIC_ERRNO(ENOTDIR), "XBOOTLDR path '%s' is not a directory.", path);
+
                 if (ret_uuid)
                         *ret_uuid = SD_ID128_NULL;
+                if (ret_devid)
+                        *ret_devid = st.st_dev;
 
                 goto found;
         }
 
-        r = verify_xbootldr("/boot", true, unprivileged_mode, ret_uuid);
+        r = verify_xbootldr("/boot", true, unprivileged_mode, ret_uuid, ret_devid);
         if (r >= 0) {
                 path = "/boot";
                 goto found;
diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h
index 6f1014db4a..16353746ae 100644
--- a/src/shared/bootspec.h
+++ b/src/shared/bootspec.h
@@ -91,5 +91,5 @@ static inline const char* boot_entry_title(const BootEntry *entry) {
         return entry->show_title ?: entry->title ?: entry->id;
 }
 
-int find_esp_and_warn(const char *path, bool unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid);
-int find_xbootldr_and_warn(const char *path, bool unprivileged_mode, char **ret_path,sd_id128_t *ret_uuid);
+int find_esp_and_warn(const char *path, bool unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid);
+int find_xbootldr_and_warn(const char *path, bool unprivileged_mode, char **ret_path,sd_id128_t *ret_uuid, dev_t *ret_devid);