Chasing symlinks is a core function that's used in a lot of places
so it deservers a less verbose names so let's rename it to chase()
and chaseat().
We also slightly change the pattern used for the chaseat() helpers
so we get chase_and_openat() and similar.
-1 was used everywhere, but -EBADF or -EBADFD started being used in various
places. Let's make things consistent in the new style.
Note that there are two candidates:
EBADF 9 Bad file descriptor
EBADFD 77 File descriptor in bad state
Since we're initializating the fd, we're just assigning a value that means
"no fd yet", so it's just a bad file descriptor, and the first errno fits
better. If instead we had a valid file descriptor that became invalid because
of some operation or state change, the other errno would fit better.
In some places, initialization is dropped if unnecessary.
We would output a sequence of concatenated JSON strings. 'jq' accepts such
output without fuss, and can even automatically build an array with --slurp/-s.
Nevertheless, parsing this format is more effort for the reader, since it's not
"standard JSON". E.g. Python's json module cannot do this out-of-the-box, but
needs some loop with json.JSONDecoder.raw_decode() and then collecting the
objects into an array. Such streaming output make sense in case of logs, where
we stream the output and it has no predefined length. But here we expect at
most a few dozen entries, so it's nicer to write normal JSON that is trivial to
parse.
I'm treating this is a bugfix and not attempting to provide compatibility
backwards. I don't think the previous format was seeing much use, and it's
trivial to adapt to the new one.
Now that the random seed is used on virtualized systems, there's no
point in having a random-seed-mode toggle switch. Let's just always
require it now, with the existing logic already being there to allow not
having it if EFI itself has an RNG. In other words, the logic for this
can now be automatic.
Let's make use of the new flag whenever we access the ESP or XBOOTLDR.
The resources we make use of in these partitions can't possibly use
symlinks (because UEFI knows no symlink concept), and they are untrusted
territory, hence under no circumstances we should be tricked into
following symlinks that shouldn't be there in the first place.
Of course, you might argue thta ESP/XBOOTLDR are VFAT and thus don#t
know symlinks. But the thing is, they don#t have to be. Firmware can
support other file systems too, and people can use efifs to gain access
to arbitrary Linux file systems from EFI. Hence, let's better be safe
than sorry.
People apparently expect the bootloader to magically know the userspace
filesystem tree and get confused when boot files aren't found because of
this. Printing the root of entry paths will hopefully make it easier for
people to understand a bad configuration.
To hopefully make it clear that the prefix should not be used inside of
a config snipped, it is greyed out and uses an extra path separator:
<gray>/root/</gray>/boot/loader/path
Let's not output a title line given that we don't output multiple
different things here anyway, and the "bootctl list" command is about
listing boot entries anyway and it's documented that way.
Having titles for sections if we have mutliple sections of output
definitely makes sense, but if there's only one kind of information we
list it's redundant clutter.
Let's better be safe than sorry, and follow symlinks in ESP + XBOOTLDR
via chase_symlinks() relative to the root of these files systems. This is
not a big issue given that ESP/XBOOTLDR are supposed to be VFAT where
symlinks don't exist, but the specs don't strictly require this, and by
loading efifs drivers in uefi it would be possible to support symlinks,
but we'd rather not have that here.
No real change in behaviour, just stricter rules to make me sleep better
at night.
This is a workaround for an issue in the memory sanitizer.
If a function is called with too many arguments, then the sanitizer
triggers the following false-positive warning:
==349==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7f8b247134a7 in json_buildv /work/build/../../src/systemd/src/shared/json.c:3213:17
#1 0x7f8b24714231 in json_build /work/build/../../src/systemd/src/shared/json.c:4117:13
#2 0x7f8b24487fa5 in show_boot_entries /work/build/../../src/systemd/src/shared/bootspec.c:1424:29
#3 0x4a6a1b in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-bootspec.c:119:16
#4 0x4c6693 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#5 0x4c5e7a in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:514:3
#6 0x4c7ce4 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:826:7
#7 0x4c7f19 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:857:3
#8 0x4b757f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6
#9 0x4e0bd2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#10 0x7f8b23ead082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
#11 0x41f69d in _start (build-out/fuzz-bootspec+0x41f69d)
Follow-up for #24541.
Fixes#24551.
It's really the file name, so name it like that. Just some renaming.
(This is preparation for later work, to extract tries-done/tries-left
info from filenames)
Inspired by #23913, let's complain if people use paths with ".."
in Type #1 bootspec entries.
Let's prefix all paths with "/" if it is missing.
Let's simplify all paths.
let's refuse paths/warn with "..".
Fixes: #23913
When the the iterator variable is declared automatically, it "inherits" the
const/non-const status from the argument. We don't need to cast a const
table to non-const. If we had a programming error and tried to modify the
string, the compiler could now catch this.
The reallocation of memory and counter incrementation is moved from
the only caller to the function. This way the callers can remain oblivious
of the BootConfig internals.
No actual code changes, just splitting out of some dev_t handling
related calls from stat-util.[ch], they are quite a number already, and
deserve their own module now I think.
Also, try to settle on the name "devnum" as the name for the concept,
instead of "devno" or "dev" or "devid". "devnum" is the name exported in
udev APIs, hence probably best to stick to that. (this just renames a
few symbols to "devum", local variables are left untouched, to make the
patch not too invasive)
No actual code changes.
This normalizes naming of functions operating on BootConfig objects.
Let's always call them boot_config_xyz(), like our usual way to name
stuff.
moreover, move the BootConfig parameter to the beginning, as it's not a
return value (which we typically move to the end of the parameter list),
but simply an object, that even happens to be initialized already.
With these changes the functions are more like our usual way to call
things, and less surprises are good.
the function contains a loop and if expressions and whatnot. Let's
define it as regular function, to make the header easier to read and let
the compiler more freedom on whether to inline this or not.
This just got too confusing for me. With this change we'll now have
_unified as common suffix for stuff loading unified kernels (i.e. type
1), and _type1 as common suffix for type1 entries.
Just some renaming.
For testing purposes I run one of my system symlinking /boot/loader/ to
/efi/loader/. This triggers annoying behaviour in boot entry
enumeration: the code ends up iterating through the 'entries' subdir of
both, thinking one was actually in the ESP and the other in XBOOTLDR,
and thus distinct. This would result in duplicate entries.
Let's address that, and filter out duplicates via their inode numbers:
never process the same inode twice. This should protect us from any
confusion effectively, regardless which inodes are symlinked (or even
bind mounted).
David Tardon
Ludwig Nussel
Daan De Meyer
Jan Janssen
Lennart Poettering
Zbigniew Jędrzejewski-Szmek
Yu Watanabe
Jason A. Donenfeld
Luca Boccassi
Victor Westerhuis