pe_section_table_from_file already checks with SECTION_TABLE_BYTES_MAX,
do the same in pe_section_table_from_base() just in case.
Originally reported on yeswehack.com as:
YWH-PGM9780-117
Follow-up for f4e081051d
Mark realName, emailAddress, and location as sensitive in JSON user
records so that they are excluded from debug log output. These fields
contain personally identifiable information that should not be leaked
in logs, which are generally more accessible than the user database
itself.
- Extract common logic to client_send_release_or_decline().
- Do not send DECLINE message on BOOTP protocol.
- Drop redundant assignment of chaddr, as it is already set by
client_message_init() -> dhcp_message_init().
- Do not assign acquired address in ciaddr field of DECLINE message,
but use Requested IP Address option.
- Broadcast DECLINE message, rather than unicast.
- Set server identifier in both cases.
Fixes#39299.
After commit "core: reuse existing dm-verity device for single
filesystem images pinned by policy" (0bd766553c), when I attach a
portable image (erofs+verity) and try to start a service, it fails with:
Partition root discovered with policy 'unprotected' but 'verity+read-only-on+growfs-off+erofs' was required, refusing.
Failed to dissect image: Operation not possible due to RF-kill
The image does have verity, in fact the RootImagePolicy= field was added
automatically.
The inconsistency between what is found at attach vs when starting the
service comes from the fact that dissect_image() is called with a
different policy as parameter and the recent shortcut added.
At attach we do this:
dissect_image(policy="*")
partition_policy_determine_fstype(policy)
partition_policy_flags_to_string(...) // mask is 0, returns 0
-> returns NULL // root_fstype_string is not set
if (root_fstype_string) // false
sym_blkid_probe_lookup_value()...
At start, as we do have the policy set, we do:
dissect_image(policy="root=verity+...+erofs:root-verity=...")
partition_policy_determine_fstype(policy)
partition_policy_flags_to_string(...) // returns 1
-> sets root_fstype_string to "erofs"
if (root_fstype_string) // true
usage = "filesystem"
Then, the service is blocked to start with the aforementioned error.
It's correct for partition_policy_determine_fstype() to set erofs in
that case, and other callers seem to expect this behavior on similar
cases, but what is not correct is to assume that this means it's a
filesystem. Usage in this case should still be unset.
Let's just always do the lookup, as that gets us the correct answer reliably
and we already did the slow part that is the probe.
The call to `sym_blkid_do_safeprobe()` is a few lines above. The call to
the lookup function isn't very expensive. blkid_probe_lookup_value()[1]
calls __blkid_probe_lookup_value(), which searches on a list[2], IIUC in
memory and no IO is used. It's a linear search of the property.
[1]: https://github.com/util-linux/util-linux/blob/0fd08f19e7a3bc37509491d06a664cfb47be7cd8/libblkid/src/probe.c#L2299
[2]: https://github.com/util-linux/util-linux/blob/0fd08f19e7a3bc37509491d06a664cfb47be7cd8/libblkid/src/probe.c#L2343
bus_cgroup_set_tasks_max_scale() used a hand-rolled percentage format
that produced values ~10x too small (e.g., "TasksMax=4.0%" instead of
"TasksMax=40.00%").
On daemon-reload, the incorrect value was re-read, silently reducing
the effective TasksMax by ~10x and causing fork rejections on systems
with high thread counts.
Fix by using the existing PERMYRIAD macros, consistent with memory
property handlers (MemoryMax, MemoryHigh, MemoryLow, etc.).
Fixes: #41009
When a DHCP client uses SendOption=82:string:..., option_append() calls
the SD_DHCP_OPTION_RELAY_AGENT_INFORMATION case which was written for
the server relay path. It casts optval to sd_dhcp_server* and calls
strlen() on its members, but optval is actually raw binary data from
the client, causing SIGSEGV. The same is applicable when option 43 and
option 77 are passed to SendOption.
Fix by checking optlen > 0 and appending the option as a plain TLV,
skipping the server-specific relay agent logic.
This is very noisy as there's a dozen of these message every
time it gets called, and it's not really an error but an
expected situation, so downgrade from info to debug
Properties are set through uevent, udev rules, or program output by IMPORT.
They may contain spurious characters and udev database parsers may be confused.
Let's refuse spurious properties.
When using systemd-mount to create a transient .mount/.automount file
for removable storage, the option to specify the idle timeout on the
commandline using **--timeout-idle-sec=SEC** is not reflected in the
generated .automount file. Instead, the idle timeout is always set to 1
second.
arg_timeout_idle_set was never set to true when passing the argument, so
arg_timeout_idle was always set to 1s.
Fixes#41007.
Co-authored-by: patrick <patrick@localhost>
The function device_add_property() handles property with NULL value as
removing the property. Let's also make an empty value is handled as
same.
Note, ENV{hoge}=="" udev property handles both NULL value and an empty string
in the same way.
We checked if the link is up only when configuring (explicit) nexthop,
but we did not checked that when configuring route which has (implicit)
nexthop.
Let's move the checks from nexthop_is_ready_to_configure() to
gateway_is_ready(), which is called for both implicit and explict
nexthops.
Fixes#40106.
Currently, we support three types of nexthop:
1. simple nexthop, which is bound to link, may have specific gateway
address,
2. blackhole nexthop, which is global configuration and is not bound to
any links,
3. group nexthop, which is also global configuration and is not bound to
any links.
Thus, gateway_is_ready() is only necessary to call for simple nexthop
case. Let's make the logic simpler.
Add a simple fuzzer that verifies our machinery for parsing user records
from JSON works as intended.
The initial corpus was created with the help of Claude, so we have a
bunch of valid user records with as much fields as possible for the
initial corpus.
The fido2_hmac_salt/fido2_hmac_credential/recovery_key fields kept
leaking memory as the array itself wasn't deallocated after deallocating
each of its elements data:
$ build-san/userdbctl -F fuzz-corpus-userdb/auth-fido2.json
...
=================================================================
==1292840==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x7f56f00e5e4b in realloc.part.0 (/lib64/libasan.so.8+0xe5e4b) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
#1 0x7f56ed869e42 in greedy_realloc ../src/basic/alloc-util.c:65
#2 0x7f56ed7ff5e9 in dispatch_fido2_hmac_salt ../src/shared/user-record.c:836
#3 0x7f56edd73cbc in sd_json_dispatch_full ../src/libsystemd/sd-json/sd-json.c:5204
#4 0x7f56edd745fc in sd_json_dispatch ../src/libsystemd/sd-json/sd-json.c:5276
#5 0x7f56ed80100b in dispatch_privileged ../src/shared/user-record.c:998
#6 0x7f56edd73cbc in sd_json_dispatch_full ../src/libsystemd/sd-json/sd-json.c:5204
#7 0x7f56edd745fc in sd_json_dispatch ../src/libsystemd/sd-json/sd-json.c:5276
#8 0x7f56ed80622c in user_record_load ../src/shared/user-record.c:1697
#9 0x000000408c15 in display_user ../src/userdb/userdbctl.c:447
#10 0x7f56ed83cc9a in dispatch_verb ../src/shared/verbs.c:137
#11 0x00000041df2b in run ../src/userdb/userdbctl.c:1908
#12 0x00000041dfbe in main ../src/userdb/userdbctl.c:1911
#13 0x7f56ec8105b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
#14 0x7f56ec810667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
#15 0x000000404a44 in _start (/home/fsumsal/repos/@systemd/systemd/build-san/userdbctl+0x404a44) (BuildId: 19e8b7e7b7038d2cea20bc18a55bea2a9e4406d5)
Direct leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x7f56f00e5e4b in realloc.part.0 (/lib64/libasan.so.8+0xe5e4b) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
#1 0x7f56ed869e42 in greedy_realloc ../src/basic/alloc-util.c:65
#2 0x7f56ed7fe779 in dispatch_fido2_hmac_credential_array ../src/shared/user-record.c:775
#3 0x7f56edd73cbc in sd_json_dispatch_full ../src/libsystemd/sd-json/sd-json.c:5204
#4 0x7f56edd745fc in sd_json_dispatch ../src/libsystemd/sd-json/sd-json.c:5276
#5 0x7f56ed80622c in user_record_load ../src/shared/user-record.c:1697
#6 0x000000408c15 in display_user ../src/userdb/userdbctl.c:447
#7 0x7f56ed83cc9a in dispatch_verb ../src/shared/verbs.c:137
#8 0x00000041df2b in run ../src/userdb/userdbctl.c:1908
#9 0x00000041dfbe in main ../src/userdb/userdbctl.c:1911
#10 0x7f56ec8105b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
#11 0x7f56ec810667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
#12 0x000000404a44 in _start (/home/fsumsal/repos/@systemd/systemd/build-san/userdbctl+0x404a44) (BuildId: 19e8b7e7b7038d2cea20bc18a55bea2a9e4406d5)
SUMMARY: AddressSanitizer: 176 byte(s) leaked in 2 allocation(s).
1 and 10000 are valid cgroup weight values, but the condition was
incorrectly excluding them:
$ echo '{"userName":"crashhostarray","cpuWeight":1}' | userdbctl -F -
<stdin>:1:42: JSON field 'cpuWeight' is not in valid range 1…10000.
$ echo '{"userName":"crashhostarray","cpuWeight":10000}' | userdbctl -F -
<stdin>:1:42: JSON field 'cpuWeight' is not in valid range 1…10000.
We don't always return d->formatted, even if it is available. And
depending on the cell type, we'd either overwrite it directly or free
first. Let's always free it upfront and then set unconditionally.
(In this case, we don't need to spend effort on preserving the
existing value. It's just a cache.) Setting the variable directly
allows many temporary variables to be eliminated.
Also use asprintf_safe() to simplify the allocation of the buffer.
This is probably a tiny bit slower than the direct allocation, but
table formatting shouldn't be a hot path.
asprintf is nice to use, but the _documented_ error return convention is
unclear:
> If memory allocation wasn't possible, or some other error occurs,
> these functions will return -1, and the contents of strp are undefined.
What exactly "undefined" means is up for debate: if it was really
undefined, the caller wouldn't be able to meaningfully clean up, because
they wouldn't know if strp is a valid pointer. So far we interpreted
"undefined" — in some parts of the code base — as "either NULL or a
valid pointer that needs to be freed", and — in other parts of the
codebase — as "always NULL". I checked glibc and musl, and they both
uncoditionally set the output pointer to NULL on failure.
There is also no information _why_ asprintf failed. It could be an
allocation error or format string error. But we just don't have this
information.
Let's add a wrapper that either returns a good string or a NULL pointer.
Since there's just one failure result, we don't need a separate return
value and an output argument and can simplify callers.
It's actually only used in one place in libsystemd and moving it even makes
libsystemd smaller (in a non-optimized build):
$ ls -l build/libsystemd.so.0.43.0*
-rwxr-xr-x 1 zbyszek zbyszek 5763336 Mar 3 09:54 build/libsystemd.so.0.43.0-old
-rwxr-xr-x 1 zbyszek zbyszek 5763216 Mar 3 09:54 build/libsystemd.so.0.43.0
Also, move the definitions in the .h file so that similar functions are
grouped together and then move the definitions around in the .c file so
that they are in the same order as in the header.
Let's make sure the parameters/error pointers into the message remain
valid as long as the reply callback is running, even if the reply
otherwise resets the pending message.
Suppress warnings like the following from clang tidy:
```
../src/boot/addon.c:11:19: error: function 'efi_main' can be made static to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors]
11 | EFIAPI EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *system_table);
| ^
```
Some warnings are suppressed simply by setting comments to ignore the warning,
some are by making global variables static, or include a suitable header.
Follow-up for ab73333c43
Methods that take numeric pid values use 0 to denote the peer,
hence let's log about 0 on PIDREF_AUTOMATIC, -1 if truly unset.
On top of libcrypt.so.2 and libcrypt.so.1, also try libcrypt.so.1.1
as a third fallback. This is used on debian alpha, and it was
reported that it is intended to ship like that, with a different
SONAME than other architectures:
https://packages.debian.org/sid/alpha/libcrypt1/filelist
Currently pcrlock won't predict PCR values that would be present at the start
of the requested location range (unless there are no events for that PCR in the
location range). This means predictions for the default range 760:940, which is
intended to start just after entering the initrd, are not actually possible to
fulfill until after the initrd is exited (or possibly even later, depending on
what other events are recorded).
Fix this by recording predictions immediately prior to processing components
after the start point.
Fixes#39946
Daan De Meyer
Zbigniew Jędrzejewski-Szmek
Luca Boccassi
Dylan M. Taylor
Yu Watanabe
Rodrigo Campos
Michael Vogt
Cyrus Xi
Nandakumar Raghavan
Lennart Poettering
ppkramer-hubandpatrick
Mike Yuan
Frantisek Sumsal
Fergus Dall