boot: Fix readdir_harder() on VirtualBox

Fixes: #22073

man/systemd-boot.xml

@@ -501,10 +501,10 @@
     considered 'good' from then on.</para>
 
     <para>The boot menu takes the 'tries left' counter into account when sorting the menu entries: entries in 'bad'
-    state are ordered at the beginning of the list, and entries in 'good' or 'indeterminate' at the end. The user can
-    freely choose to boot any entry of the menu, including those already marked 'bad'. If the menu entry to boot is
-    automatically determined, this means that 'good' or 'indeterminate' entries are generally preferred (as the bottom
-    item of the menu is the one booted by default), and 'bad' entries will only be considered if there are no 'good' or
+    state are ordered towards the end of the list, and entries in 'good' or 'indeterminate' towards the beginning.
+    The user can freely choose to boot any entry of the menu, including those already marked 'bad'. If the menu entry
+    to boot is automatically determined, this means that 'good' or 'indeterminate' entries are generally preferred as
+    boot entries are tried in sort order, and 'bad' entries will only be considered if there are no 'good' or
     'indeterminate' entries left.</para>
 
     <para>The <citerefentry><refentrytitle>kernel-install</refentrytitle><manvolnum>8</manvolnum></citerefentry> kernel

src/boot/efi/boot.c

@@ -1492,7 +1492,6 @@ static void config_entry_add_from_file(
 
         entry->device = device;
         entry->id = xstrdup(file);
-        StrLwr(entry->id);
 
         config_add_entry(config, entry);
 
@@ -1578,7 +1577,7 @@ static void config_load_entries(
                 _cleanup_freepool_ CHAR8 *content = NULL;
 
                 err = readdir_harder(entries_dir, &f, &f_size);
-                if (f_size == 0 || EFI_ERROR(err))
+                if (EFI_ERROR(err) || !f)
                         break;
 
                 if (f->FileName[0] == '.')
@@ -1603,13 +1602,24 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) {
         assert(a);
         assert(b);
 
-        /* Order entries that have no tries left to the beginning of the list */
+        /* Order entries that have no tries left towards the end of the list. They have
+         * proven to be bad and should not be selected automatically. */
         if (a->tries_left != 0 && b->tries_left == 0)
-                return 1;
-        if (a->tries_left == 0 && b->tries_left != 0)
                 return -1;
+        if (a->tries_left == 0 && b->tries_left != 0)
+                return 1;
 
-        r = strverscmp_improved(a->id, b->id);
+        r = strcasecmp_ptr(a->title ?: a->id, b->title ?: b->id);
+        if (r != 0)
+                return r;
+
+        /* Sort by machine id now so that different installations don't interleave their versions. */
+        r = strcasecmp_ptr(a->machine_id, b->machine_id);
+        if (r != 0)
+                return r;
+
+        /* Reverse version comparison order so that higher versions are preferred. */
+        r = strverscmp_improved(b->version, a->version);
         if (r != 0)
                 return r;
 
@@ -1617,19 +1627,20 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) {
             b->tries_left == UINTN_MAX)
                 return 0;
 
-        /* If both items have boot counting, and otherwise are identical, put the entry with more tries left last */
+        /* If both items have boot counting, and otherwise are identical, put the entry with more tries left first */
         if (a->tries_left > b->tries_left)
-                return 1;
-        if (a->tries_left < b->tries_left)
                 return -1;
+        if (a->tries_left < b->tries_left)
+                return 1;
 
         /* If they have the same number of tries left, then let the one win which was tried fewer times so far */
         if (a->tries_done < b->tries_done)
-                return 1;
-        if (a->tries_done > b->tries_done)
                 return -1;
+        if (a->tries_done > b->tries_done)
+                return 1;
 
-        return 0;
+        /* As a last resort, use the id (file name). */
+        return strverscmp_improved(a->id, b->id);
 }
 
 static UINTN config_entry_find(Config *config, const CHAR16 *needle) {
@@ -1638,7 +1649,7 @@ static UINTN config_entry_find(Config *config, const CHAR16 *needle) {
         if (!needle)
                 return IDX_INVALID;
 
-        for (INTN i = config->entry_count - 1; i >= 0; i--)
+        for (UINTN i = 0; i < config->entry_count; i++)
                 if (MetaiMatch(config->entries[i]->id, (CHAR16*) needle))
                         return i;
 
@@ -1673,9 +1684,8 @@ static void config_default_entry_select(Config *config) {
                 return;
         }
 
-        /* select the last suitable entry */
-        i = config->entry_count;
-        while (i--) {
+        /* Select the first suitable entry. */
+        for (i = 0; i < config->entry_count; i++) {
                 if (config->entries[i]->type == LOADER_AUTO || config->entries[i]->call)
                         continue;
                 config->idx_default = i;
@@ -1830,8 +1840,6 @@ static ConfigEntry *config_entry_add_loader(
                 .tries_left = UINTN_MAX,
         };
 
-        StrLwr(entry->id);
-
         config_add_entry(config, entry);
         return entry;
 }
@@ -2011,7 +2019,7 @@ static void config_entry_add_linux(
                 CHAR8 *key, *value;
 
                 err = readdir_harder(linux_dir, &f, &f_size);
-                if (f_size == 0 || EFI_ERROR(err))
+                if (EFI_ERROR(err) || !f)
                         break;
 
                 if (f->FileName[0] == '.')
@@ -2326,12 +2334,14 @@ static void config_load_all_entries(
         /* Similar, but on any XBOOTLDR partition */
         config_load_xbootldr(config, loaded_image->DeviceHandle);
 
+        /* Add these now, so they get sorted with the rest. */
+        config_entry_add_osx(config);
+        config_entry_add_windows(config, loaded_image->DeviceHandle, root_dir);
+
         /* sort entries after version number */
         sort_pointer_array((void **) config->entries, config->entry_count, (compare_pointer_func_t) config_entry_compare);
 
         /* if we find some well-known loaders, add them to the end of the list */
-        config_entry_add_osx(config);
-        config_entry_add_windows(config, loaded_image->DeviceHandle, root_dir);
         config_entry_add_loader_auto(config, loaded_image->DeviceHandle, root_dir, NULL,
                                      L"auto-efi-shell", 's', L"EFI Shell", L"\\shell" EFI_MACHINE_TYPE_NAME ".efi");
         config_entry_add_loader_auto(config, loaded_image->DeviceHandle, root_dir, loaded_image_path,

src/boot/efi/util.c

@@ -592,7 +592,12 @@ EFI_STATUS readdir_harder(
          * the specified buffer needs to be freed by caller, after final use. */
 
         if (!*buffer) {
-                sz = offsetof(EFI_FILE_INFO, FileName) /* + 256 */;
+                /* Some broken firmware violates the EFI spec by still advancing the readdir
+                 * position when returning EFI_BUFFER_TOO_SMALL, effectively skipping over any files when
+                 * the buffer was too small. Therefore, start with a buffer that should handle FAT32 max
+                 * file name length.
+                 * As a side effect, most readdir_harder() calls will now be slightly faster. */
+                sz = sizeof(EFI_FILE_INFO) + 256 * sizeof(CHAR16);
                 *buffer = xallocate_pool(sz);
                 *buffer_size = sz;
         } else

src/core/bpf-lsm.c

@@ -45,10 +45,11 @@ static bool bpf_can_link_lsm_program(struct bpf_program *prog) {
         assert(prog);
 
         link = sym_bpf_program__attach_lsm(prog);
-        if (!link)
-                return -ENOMEM;
 
-        return 1;
+        /* If bpf_program__attach_lsm fails the resulting value stores libbpf error code instead of memory
+         * pointer. That is the case when the helper is called on architectures where BPF trampoline (hence
+         * BPF_LSM_MAC attach type) is not supported. */
+        return sym_libbpf_get_error(link) == 0;
 }
 
 static int prepare_restrict_fs_bpf(struct restrict_fs_bpf **ret_obj) {
@@ -166,9 +167,9 @@ int lsm_bpf_supported(void) {
         if (r < 0)
                 return supported = 0;
 
-        r = bpf_can_link_lsm_program(obj->progs.restrict_filesystems);
-        if (r < 0) {
-                log_warning_errno(r, "Failed to link BPF program. Assuming BPF is not available: %m");
+        if (!bpf_can_link_lsm_program(obj->progs.restrict_filesystems)) {
+                log_warning_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                  "Failed to link BPF program. Assuming BPF is not available");
                 return supported = 0;
         }
 

src/shared/bpf-link.c

@@ -32,12 +32,12 @@ int bpf_serialize_link(FILE *f, FDSet *fds, const char *key, struct bpf_link *li
 }
 
 struct bpf_link *bpf_link_free(struct bpf_link *link) {
-
-        /* Avoid a useless dlopen() if link == NULL */
-        if (!link)
-                return NULL;
-
-        (void) sym_bpf_link__destroy(link);
+        /* If libbpf wasn't dlopen()ed, sym_bpf_link__destroy might be unresolved (NULL), so let's not try to
+         * call it if link is NULL. link might also be a non-null "error pointer", but such a value can only
+         * originate from a call to libbpf, but that means that libbpf is available, and we can let
+         * bpf_link__destroy() handle it. */
+        if (link)
+                (void) sym_bpf_link__destroy(link);
 
         return NULL;
 }