Four *_attach_event helpers swallow sd_event_default() failures and
return success while leaving obj->event NULL:
sd_dhcp_client_attach_event (sd-dhcp-client.c)
sd_dhcp6_client_attach_event (sd-dhcp6-client.c)
sd_ndisc_attach_event (sd-ndisc.c)
sd_radv_attach_event (sd-radv.c)
Each contains the same copy-pasted typo in the NULL-event branch:
r = sd_event_default(&obj->event);
if (r < 0)
return 0; /* swallows -ENOMEM / -ECHILD */
The caller is told the attach succeeded, but obj->event is still
NULL. A subsequent *_start() then trips assert_return(obj->event,
-EINVAL) and returns a spurious -EINVAL (or aborts on assert-abort
builds), and any consumer of *_get_event() dereferences NULL.
The sibling helper sd_dhcp_server_attach_event already uses the
correct pattern:
r = sd_event_default(&server->event);
if (r < 0)
return r;
Fix by returning r instead of 0 at each of the four sites so the
sd_event_default() errno is propagated to the caller and ownership
stays with the caller.
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
apply_syscall_filter() unconditionally inserts the write() syscall
into c->syscall_filter when exec_fd or handoff_timestamp_fd is in
use, so the parent can receive the exec status / handoff timestamp
from the child. When the unit configured a positive
SystemCallFilter= allow-list that deliberately omits write(), the
resulting widening of the operator's policy happens silently with
no trace in the journal.
Emit a log_debug() before the seccomp_filter_set_add_by_name() call
when syscall_allow_list is true, so the widening is at least
observable to operators inspecting the unit's debug log.
While here, document that mutating c->syscall_filter through a
'const ExecContext *c' is intentional: apply_syscall_filter() runs
only in the post-fork child, which owns a private copy of the
address space, so the hashmap change is never observed by the
manager.
No functional change for the allow-list itself; write() is still
added exactly as before.
Fixes: 84b79215cc ("core: do not filter out write() if required in the very late stage")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
apply_root_directory() calls chroot() but never pairs it with a
chdir("/"). Moving cwd into the new root is left to the subsequent
apply_working_directory() call. That delegation breaks when the unit
sets WorkingDirectory=-/path (working_directory_missing_ok):
apply_root_directory() apply_working_directory()
chroot(new_root) --> chdir(wd) fails (ENOENT)
return 0 return 0 /* missing_ok */
chroot(2) changes only the process root, not cwd. With cwd still
resolving to the pre-chroot host dentry, the process runs with the
kernel root pointing at the new root while relative-path accesses
and /proc/self/cwd reach paths outside RootDirectory=.
Only the chroot-only branch (exec_needs_mount_namespace() == false)
is affected. The mount-namespace branch uses pivot_root(), which
restructures the mount tree and makes the pre-chroot dentry
unreachable.
Fix by issuing chdir("/") immediately after the successful chroot()
in apply_root_directory() and propagating any failure with
EXIT_CHROOT, matching the standard chroot+chdir idiom and making
the chroot self-contained regardless of how
apply_working_directory() later handles a missing directory.
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
sd_dhcp6_client_add_option() and sd_dhcp6_client_add_vendor_option()
store the caller's sd_dhcp6_option in an ordered container whose value
destructor is sd_dhcp6_option_unref, registered via
DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR in dhcp6_option_hash_ops. The
container therefore owns exactly one ref per stored slot.
Both helpers guarded the ref bump only on r < 0:
r = ordered_{hashmap,set}_ensure_put(..., v);
if (r < 0)
return r;
sd_dhcp6_option_ref(v);
ordered_hashmap_ensure_put() (and the ordered_set wrapper) forward the
underlying hashmap_put_boldly tri-state verbatim: 1 on fresh insert, 0
when the identical key+value pair is already stored (no new slot
allocated), <0 on error. The r == 0 path fell through and bumped the
refcount without a corresponding slot.
The extra ref is permanently stranded once the container's destructor
runs. In a long-running networkd that reapplies .network files on
reload this leaks one sd_dhcp6_option allocation per duplicate-add.
Fix by tightening the guard to r <= 0 in both helpers, so
sd_dhcp6_option_ref(v) is reached only on a genuine new slot (r == 1).
Propagate the tri-valued result by returning r from both helpers
instead of a hard-coded constant.
sd_dhcp6_client_add_vendor_option() is a public libsystemd sd_ API;
its success return changes from a constant 1 to {0, 1}.
sd_dhcp6_client_add_option() now also propagates the tri-valued
result (previously a hard-coded 0 on success), giving the two parallel
helpers identical return contracts.
The sole non-fuzz in-tree callers are both in dhcp6_configure()
(src/network/networkd-dhcp6.c), which treat any r >= 0 as success and
do not distinguish 0 from 1, so the contract widening is caller-safe.
Fixes: e7d5fe17db ("DHCP client: make SendOption work for DHCPv6 too.")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
In the PARTITION_ROOT_VERITY_SIG / PARTITION_USR_VERITY_SIG branch of
dissect_image(), the call to acquire_sig_for_roothash() passes the
partition offset and size as
acquire_sig_for_roothash(fd,
start * 512,
size * 512,
&root_hash, NULL);
where start and size are blkid_loff_t (int64_t). The literal 512 is
int, so the multiplication is performed in int64_t and overflows for
LBA values in (INT64_MAX/512, INT64_MAX]. The overflowed signed result
is then converted to the uint64_t parameters of
acquire_sig_for_roothash(), yielding a near-UINT64_MAX
partition_offset that slips past the sole offset guard (an exact
== UINT64_MAX sentinel) and reaches pread() with a corrupt disk
offset. An overflowed partition_size would be rejected by the 4 MiB
EFBIG check at src/shared/dissect-image.c:710-712 before pread(); the
dangerous scenario is a corrupt offset combined with a legitimate
small size.
The three sibling call sites at src/shared/dissect-image.c:1284,
1589-1590 and 1671-1672 all cast to uint64_t before multiplying by
512; only this call site was missed.
Fix by casting start and size to uint64_t before the multiplication,
matching the sibling sites and making the arithmetic well-defined.
Fixes: 98ca65c36a ("dissect: check that roothash in signature matches before selecting partition")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
In acquire_sig_for_roothash() the pread() failure branch returns
-ENOMEM, which is copy-pasted from the malloc() check immediately
above it:
_cleanup_free_ char *buf = new(char, partition_size+1);
if (!buf)
return -ENOMEM;
ssize_t n = pread(fd, buf, partition_size, partition_offset);
if (n < 0)
return -ENOMEM;
pread() sets errno to the actual I/O failure (EIO, EINTR, EBADF,
...). Aliasing those to -ENOMEM misleads dissect_log_error() and
any caller that branches on the returned code; verity signature
lookup failures get reported as out-of-memory.
Fix by returning -errno from the pread() failure branch. The
malloc() branch above is correct and unchanged.
Fixes: 98ca65c36a ("dissect: check that roothash in signature matches before selecting partition")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
mountfsd_make_directory() opens the parent directory after a
successful path_extract_filename() call, but reports failure using
the stale path_extract_filename() return value:
r = path_extract_filename(path, &dirname);
if (r < 0)
return log_debug_errno(r, ...);
...
_cleanup_close_ int fd = open(parent, O_DIRECTORY|O_CLOEXEC);
if (fd < 0)
return log_debug_errno(r, "Failed to open '%s': %m", parent);
At the open() check site r holds the non-negative return of
path_extract_filename(). log_debug_errno() with a non-negative
first argument returns 0, so the function reports success even
though open() failed. Callers that pass a non-NULL ret_directory_fd
then consume an uninitialized fd value, and the diagnostic message
is suppressed because log_debug_errno(0, ...) emits nothing.
Fix by passing errno instead of r, matching the convention used in
mountfsd_mount_image() and mountfsd_mount_directory() in the same
file. The real open() failure (ENOENT, EACCES, ENOTDIR, EMFILE, ...)
now propagates to the caller and the log message is preserved.
Fixes: 1be8caa6be ("importd: support unpacking tarballs to foreign UID range")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
RSA PKCS#1 v1.5 is vulnerable to Bleichenbacher-style padding oracle
attacks, albeit very difficult and unlikely to actually happen in the
real world. Still for hardedning, switch new enrollments to RSA-OAEP,
with SHA-256 preferred and SHA-1 as fallback (probed at enrollment time,
since e.g. SoftHSM only accepts SHA-1, and older token might as well).
The actual padding scheme used to wrap a given key is recorded as a new
optional 'pkcs11-padding' / 'padding' field in the LUKS2 token JSON and
the homed user record. Decryption defaults to PKCS#1 v1.5 when absent so
existing enrollments keep working.
`sysupdate --json=short components` lists the components known to
sysupdate; these are the components which something like `updatectl`
will try to update.
The `default` component represents the host, and is meant to be listed
if transfer definitions exist in (for example) `/etc/sysupdate.d`
corresponding to the host OS. This then corresponds to `TARGET_HOST` in
`updatectl` and causes it to try updating that target.
The logic for working out whether the `default` component was present
essentially boiled down to “does `{/run,/etc,/usr/lib}/sysupdate.d`
exist”, and it didn’t check whether a `.transfer` or `.conf` file
actually existed in the config directory.
This is quite the corner case, but becomes more evident on systems where
sysupdate is being used to update a portable service but not the main
OS. At that point, if `/etc/sysupdate.d` exists empty (for some reason),
`updatectl` falls over because it starts trying to update the host OS
without any configuration to do so.
So, modify `sysupdate` to more fully load the available configuration
when listing components, and query it a bit more deeply to check whether
a default component exists.
If `sysupdate` is called with various command line arguments to affect
how its configuration is loaded, do *not* say that a default component
exists, as these arguments essentially anull the possibility of a
default being used in that process.
Add an integration test based on the reproducer provided by the issue
reporter. This test has been tested to fail if the changes to
`sysupdate.c` aren’t applied — if so, the second call to `sysupdate
components` would return
`{"default":true,"components":["some-component"]}`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: https://github.com/systemd/systemd/issues/41501
manager_luo_serialize_fd_stores() refuses to serialize a unit fd store
entry that holds a child LUO session named like PID 1's own ("systemd"),
to stop a service from hijacking PID 1's reserved session namespace
across kexec. That guard had no test coverage.
Add a test-luo store-hijack/check-hijack subcommand pair: on the first
boot a system service preserves a child LUO session named "systemd" in
its fd store; after kexec the test asserts the entry was not restored --
the unit's NFileDescriptorStore is 0, and check-hijack, run as the unit's
own second-boot ExecStart, confirms the hijack fd is absent from its
restored LISTEN_FDNAMES -- proving PID 1 skipped it during serialization.
The restore-side guards (corrupt mapping, reserved token 0, invalid unit
name, missing child session) are intentionally not covered: they only run
against PID 1's own "systemd" session built by luo_preserve_fd_stores(),
which a cooperating userspace helper cannot corrupt without racing or
displacing PID 1 (it single-owns /dev/liveupdate at shutdown). Triggering
them reliably would need kernel fault injection.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
`sysupdate --json=short components` lists the components known to
sysupdate; these are the components which something like `updatectl`
will try to update.
The `default` component represents the host, and is meant to be listed
if transfer definitions exist in (for example) `/etc/sysupdate.d`
corresponding to the host OS. This then corresponds to `TARGET_HOST` in
`updatectl` and causes it to try updating that target.
The logic for working out whether the `default` component was present
essentially boiled down to “does `{/run,/etc,/usr/lib}/sysupdate.d`
exist”, and it didn’t check whether a `.transfer` or `.conf` file
actually existed in the config directory.
This is quite the corner case, but becomes more evident on systems where
sysupdate is being used to update a portable service but not the main
OS. At that point, if `/etc/sysupdate.d` exists empty (for some reason),
`updatectl` falls over because it starts trying to update the host OS
without any configuration to do so.
So, modify `sysupdate` to more fully load the available configuration
when listing components, and query it a bit more deeply to check whether
a default component exists.
If `sysupdate` is called with various command line arguments to affect
how its configuration is loaded, do *not* say that a default component
exists, as these arguments essentially anull the possibility of a
default being used in that process.
Add an integration test based on the reproducer provided by the issue
reporter. This test has been tested to fail if the changes to
`sysupdate.c` aren’t applied — if so, the second call to `sysupdate
components` would return
`{"default":true,"components":["some-component"]}`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: https://github.com/systemd/systemd/issues/41501
Optionally prevent `context_read_definitions()` erroring out if zero
transfer definitions were found.
This commit makes no functional changes (the flag is always passed to
calls to `context_make_offline()` for the moment), but the new flag will be
used in the following commit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: https://github.com/systemd/systemd/issues/41501
This commit makes no functional changes, but the new flags will be
used and extended in the following commit. We need a flags variable to
avoid having two bool arguments, which would be confusing.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: https://github.com/systemd/systemd/issues/41501
This irked me for a while. Let's not scan for strings stupidly, but
properly parse PE files to find the magic marker.
It's easy with our own PE APIs, hence we should do it.
This moves logging to the callers (previously, this was all mixed up).
Let's return -EBADMSG if the PE headers reference stuff missing in the
file, regardless if that's because the offsets are larger than SSIZE_MAX
or just larger than the file size. We generally use EBADMSG for all
cases we deem the file to not be a conformant PE file, and these two
cases are the same. Hence, let's be systematic here.
This is supposed to be useful to mark an interactive user login as a
"break glass" event in the measurement logs, i.e. as in many typically
headless scenerios this indicates debug access or similar.
This allows us to have a separate, more permissive, policy for
cancelling ongoing sysupdate jobs. The new default policy for
cancellation actions is to allow them for the active user, without admin
authentication, because typically the user can just pull the plug on the
computer to cancel a job anyway.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: https://github.com/systemd/systemd/issues/38568
EFI_GUID requires 4-byte alignment due to its uint32_t Data1 field, but
callers may pass pointers at arbitrary offsets into serialized EFI
variable buffers (e.g. bootctl walking BootXXXX entries). UBSan flagged
the misaligned member access; the old comment claiming the struct was
packed was wrong. Copy the bytes into an aligned local first.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Ensure that fnmatch traversal doesn't go off the hwdb.bin in case of
a corrupted file
Originally reported on yeswehack.com as #YWH-PGM9780-70
For the unit test:
Co-developed-by: GitHub Copilot <github-copilot[bot]@users.noreply.github.com>
Previously, DHCPNAK was sent only when the client is in INIT-REBOOT
state. But, on selecting or renewing, the request is directed to a
specific server, so we can safely reply with DHCPNAK.
Also, verify existing bound lease even when there is no static lease for
the client.
This also makes the conditions in dhcp_server_send_message() uses
the message that will be sent, rather than we received.
This does not change basic functionality, but changes/fixes several
minor behaviors, e.g.
- fix when the broadcast flag assignment,
- set server identifier in DHCPFORCERENEW.
This is mostly refactoring. This does not change basic behavior, but
changes/fixes some minor/corner cases, e.g.
- extend the minimum default lease time from 1 second to 30 seconds, as
1 second is too short and causes the network unstable (though 30
seconds is stll too short, but hopefully that does not make the
network unstable).
- error code on broken/malicious message received may be changed.
Chris Mason
Lennart Poettering
Luca Boccassi
Yu Watanabe
Daan De Meyer
Rocker Zhang
Philip Withnall