1
0
mirror of https://github.com/systemd/systemd synced 2026-03-29 19:24:50 +02:00

Compare commits

...

5 Commits

Author SHA1 Message Date
Chris Down
d1ae5e26c4
nspawn: Fix broken host links for container journals (#39727)
Commit 88252ca changed nspawn to always run from a temporary mount
directory (e.g., /tmp/nspawn-root-XXXXXX). This was a good
simplification for mount logic, but it unintentionally broke the
--link-journal feature.

The setup_journal() helper was subsequently passed this ephemeral path
instead of the persistent machine path (from --directory= or --image=).
This caused the host to create broken symlinks pointing to a temporary
directory that would soon be gone.

Fix this by storing the original path and plumbing it through to
setup_journal().

All other mount-related logic in outer_child() continues to use the
temporary `directory` variable.

Fixes: #39472
2025-11-21 23:42:23 +08:00
Chris Down
887f54adce nspawn: Add integration test for --link-journal 2025-11-21 21:04:34 +08:00
Chris Down
11eebc2357 nspawn: Fix broken host links for container journals
Commit 88252ca changed nspawn to always run from a temporary mount
directory (e.g., /tmp/nspawn-root-XXXXXX). This was a good
simplification for mount logic, but it unintentionally broke the
--link-journal feature.

The setup_journal() helper was subsequently passed this ephemeral path
instead of the persistent machine path (from --directory= or --image=).
This caused the host to create broken symlinks pointing to a temporary
directory that would soon be gone.

Fix this by storing the original path and plumbing it through to
setup_journal().

All other mount-related logic in outer_child() continues to use the
temporary `directory` variable.

Fixes: #39472
2025-11-21 21:04:34 +08:00
Daan De Meyer
2691e7558b run0: Add note about processes having privileges over --empower sessions 2025-11-21 13:08:50 +01:00
Daan De Meyer
cf063b8a1c sd-bus: Exit event loop with error code instead of EXIT_FAILURE
Instead of failing the event loop with a generic EXIT_FAILURE
error code when exit-on-disconnect is used, let's propagate the
error code instead of swallowing it.

Whereas previously sd_event_loop() would always fail with exit code
'1' when exit-on-disconnect is used with an sd-bus instance registered
with the event loop that encounters a failure, now we'll correctly
propagate the error to sd_event_loop() that caused sd-bus to fail and
exit the event loop. Additionally, the error is now also properly
propagated to outstanding reply callbacks for async dbus calls started
with sd_bus_call_async() and friends, whereas before we always used
ETIMEDOUT for these calls which is extremely confusing for users.

Why is this confusing? We always start sd-bus instances asynchronously,
in other words, sd_bus_start() will not actually wait until the bus instance
is connected, but it'll happen in the background, either driven by the first
sd_bus_call() when there is no event loop or by sd-event when there is an
event loop attached to the sd-bus instance. Assuming an event loop is attached,
when we fail to connect to the bus, the sd-bus instance will close down and the
first async method call we queued will fail with ETIMEDOUT. Nowhere in this process
do we inform the user that we failed to connect to the bus because of e.g. a permission
error, except for a debug log message.

By propagating the error to sd_event_exit() if exit-on-disconnect is enabled
and always propagating it to outstanding reply callbacks, debugging failures
becomes much easier as users will now get the actual error code causing the
bus instance to close down instead of ETIMEDOUT and 1 respectively.
2025-11-21 09:57:51 +01:00
7 changed files with 249 additions and 49 deletions

View File

@ -299,6 +299,10 @@
group as a supplemental group (for which all polkit actions are allowed by default), but other
privileges may be granted in the future as well when using this option.</para>
<para>Note that other (unprivileged) processes of the selected user will have privileges over the
invoked process. Consider not using this option in an environment where there might be malicious
processes running as the selected user.</para>
<xi:include href="version-info.xml" xpointer="v259"/></listitem>
</varlistentry>

View File

@ -140,7 +140,7 @@ static int default_request_name_handler(
"Unable to request name, failing connection: %s",
sd_bus_message_get_error(m)->message);
bus_enter_closing(sd_bus_message_get_bus(m));
bus_enter_closing(sd_bus_message_get_bus(m), -sd_bus_message_get_errno(m));
return 1;
}
@ -164,12 +164,12 @@ static int default_request_name_handler(
case BUS_NAME_EXISTS:
log_debug("Requested service name already owned, failing connection.");
bus_enter_closing(sd_bus_message_get_bus(m));
bus_enter_closing(sd_bus_message_get_bus(m), -EEXIST);
return 1;
}
log_debug("Unexpected response from RequestName(), failing connection.");
bus_enter_closing(sd_bus_message_get_bus(m));
bus_enter_closing(sd_bus_message_get_bus(m), -EPROTO);
return 1;
}
@ -294,7 +294,7 @@ static int default_release_name_handler(
"Unable to release name, failing connection: %s",
sd_bus_message_get_error(m)->message);
bus_enter_closing(sd_bus_message_get_bus(m));
bus_enter_closing(sd_bus_message_get_bus(m), -sd_bus_message_get_errno(m));
return 1;
}
@ -318,7 +318,7 @@ static int default_release_name_handler(
}
log_debug("Unexpected response from ReleaseName(), failing connection.");
bus_enter_closing(sd_bus_message_get_bus(m));
bus_enter_closing(sd_bus_message_get_bus(m), -EPROTO);
return 1;
}

View File

@ -164,6 +164,7 @@ typedef struct sd_bus {
BusState state;
int input_fd, output_fd;
int inotify_fd;
int exit_code;
int message_version;
int message_endian;
@ -403,6 +404,6 @@ int bus_maybe_reply_error(sd_bus_message *m, int r, const sd_bus_error *e);
return sd_bus_error_set_errno(error, r); \
} while (false)
void bus_enter_closing(sd_bus *bus);
void bus_enter_closing(sd_bus *bus, int exit_code);
void bus_set_state(sd_bus *bus, BusState state);

View File

@ -257,6 +257,7 @@ _public_ int sd_bus_new(sd_bus **ret) {
.input_fd = -EBADF,
.output_fd = -EBADF,
.inotify_fd = -EBADF,
.exit_code = EXIT_FAILURE,
.message_version = 1,
.creds_mask = SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME,
.accept_fd = true,
@ -1813,13 +1814,14 @@ _public_ sd_bus* sd_bus_flush_close_unref(sd_bus *bus) {
return sd_bus_close_unref(bus);
}
void bus_enter_closing(sd_bus *bus) {
void bus_enter_closing(sd_bus *bus, int exit_code) {
assert(bus);
if (!IN_SET(bus->state, BUS_WATCH_BIND, BUS_OPENING, BUS_AUTHENTICATING, BUS_HELLO, BUS_RUNNING))
return;
bus_set_state(bus, BUS_CLOSING);
bus->exit_code = exit_code;
}
/* Define manually so we can add the PID check */
@ -2189,9 +2191,10 @@ _public_ int sd_bus_send(sd_bus *bus, sd_bus_message *_m, uint64_t *ret_cookie)
r = bus_write_message(bus, m, &idx);
if (ERRNO_IS_NEG_DISCONNECT(r)) {
bus_enter_closing(bus);
bus_enter_closing(bus, r);
return -ECONNRESET;
} else if (r < 0)
}
if (r < 0)
return r;
if (idx < BUS_MESSAGE_SIZE(m)) {
@ -2486,7 +2489,7 @@ _public_ int sd_bus_call(
r = bus_read_message(bus);
if (r < 0) {
if (ERRNO_IS_DISCONNECT(r)) {
bus_enter_closing(bus);
bus_enter_closing(bus, r);
r = -ECONNRESET;
}
@ -2521,7 +2524,7 @@ _public_ int sd_bus_call(
r = dispatch_wqueue(bus);
if (r < 0) {
if (ERRNO_IS_DISCONNECT(r)) {
bus_enter_closing(bus);
bus_enter_closing(bus, r);
r = -ECONNRESET;
}
@ -3110,14 +3113,14 @@ static int bus_exit_now(sd_bus *bus, sd_event *event) {
event = bus->event;
if (event)
return sd_event_exit(event, EXIT_FAILURE);
return sd_event_exit(event, bus->exit_code);
exit(EXIT_FAILURE);
assert_not_reached();
}
static int process_closing_reply_callback(sd_bus *bus, BusReplyCallback *c) {
_cleanup_(sd_bus_error_free) sd_bus_error error_buffer = SD_BUS_ERROR_NULL;
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL, error_buffer = SD_BUS_ERROR_NULL;
_cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
sd_bus_slot *slot;
int r;
@ -3125,11 +3128,12 @@ static int process_closing_reply_callback(sd_bus *bus, BusReplyCallback *c) {
assert(bus);
assert(c);
r = bus_message_new_synthetic_error(
bus,
c->cookie,
&SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_NO_REPLY, "Connection terminated"),
&m);
(void) sd_bus_error_set_errnof(
&error,
bus->exit_code < 0 ? bus->exit_code : -ETIMEDOUT,
"Connection terminated");
r = bus_message_new_synthetic_error(bus, c->cookie, &error, &m);
if (r < 0)
return r;
@ -3293,7 +3297,7 @@ static int bus_process_internal(sd_bus *bus, sd_bus_message **ret) {
}
if (ERRNO_IS_NEG_DISCONNECT(r)) {
bus_enter_closing(bus);
bus_enter_closing(bus, r);
r = 1;
} else if (r < 0)
return r;
@ -3427,7 +3431,7 @@ _public_ int sd_bus_flush(sd_bus *bus) {
for (;;) {
r = dispatch_wqueue(bus);
if (ERRNO_IS_NEG_DISCONNECT(r)) {
bus_enter_closing(bus);
bus_enter_closing(bus, r);
return -ECONNRESET;
} else if (r < 0)
return r;
@ -3478,17 +3482,17 @@ static int add_match_callback(
sd_bus_slot *match_slot = ASSERT_PTR(userdata);
bool failed = false;
int r;
int r = 0;
assert(m);
sd_bus_slot_ref(match_slot);
if (sd_bus_message_is_method_error(m, NULL)) {
log_debug_errno(sd_bus_message_get_errno(m),
"Unable to add match %s, failing connection: %s",
match_slot->match_callback.match_string,
sd_bus_message_get_error(m)->message);
r = log_debug_errno(sd_bus_message_get_errno(m),
"Unable to add match %s, failing connection: %s",
match_slot->match_callback.match_string,
sd_bus_message_get_error(m)->message);
failed = true;
} else
@ -3518,7 +3522,7 @@ static int add_match_callback(
bus->current_userdata = userdata;
} else {
if (failed) /* Generic failure handling: destroy the connection */
bus_enter_closing(sd_bus_message_get_bus(m));
bus_enter_closing(sd_bus_message_get_bus(m), r);
r = 1;
}
@ -3649,7 +3653,7 @@ static int io_callback(sd_event_source *s, int fd, uint32_t revents, void *userd
r = sd_bus_process(bus, NULL);
if (r < 0) {
log_debug_errno(r, "Processing of bus failed, closing down: %m");
bus_enter_closing(bus);
bus_enter_closing(bus, r);
}
return 1;
@ -3662,7 +3666,7 @@ static int time_callback(sd_event_source *s, uint64_t usec, void *userdata) {
r = sd_bus_process(bus, NULL);
if (r < 0) {
log_debug_errno(r, "Processing of bus failed, closing down: %m");
bus_enter_closing(bus);
bus_enter_closing(bus, r);
}
return 1;
@ -3714,7 +3718,7 @@ static int prepare_callback(sd_event_source *s, void *userdata) {
fail:
log_debug_errno(r, "Preparing of bus events failed, closing down: %m");
bus_enter_closing(bus);
bus_enter_closing(bus, r);
return 1;
}

View File

@ -33,7 +33,7 @@ static int method_exit(sd_bus_message *m, void *userdata, sd_bus_error *ret_erro
ASSERT_OK(sd_bus_reply_method_return(m, NULL));
/* Simulate D-Bus going away to test the bus_exit_now() path with exit_on_disconnect set */
bus_enter_closing(sd_bus_message_get_bus(m));
bus_enter_closing(sd_bus_message_get_bus(m), EXIT_FAILURE);
return 0;
}

View File

@ -2551,12 +2551,14 @@ static int setup_hostname(void) {
return 0;
}
static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range) {
static int setup_journal(const char *root_dir, const char *persistent_path, uid_t chown_uid, uid_t chown_range) {
_cleanup_free_ char *d = NULL;
sd_id128_t this_id;
bool try;
int r;
assert(root_dir);
/* Don't link journals in ephemeral mode */
if (arg_ephemeral)
return 0;
@ -2566,6 +2568,17 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range
try = arg_link_journal_try || arg_link_journal == LINK_AUTO;
if (!persistent_path) {
if (try) {
log_debug("No persistent path available, skipping journal linking.");
return 0;
} else
return log_error_errno(
SYNTHETIC_ERRNO(ENOENT),
"Journal linking requested but no persistent directory available. "
"Use --directory, or --link-journal=auto/try-guest/try-host for optional linking.");
}
r = sd_id128_get_machine(&this_id);
if (r < 0)
return log_error_errno(r, "Failed to retrieve machine ID: %m");
@ -2579,7 +2592,7 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range
}
FOREACH_STRING(dirname, "/var", "/var/log", "/var/log/journal") {
r = userns_mkdir(directory, dirname, 0755, 0, 0);
r = userns_mkdir(root_dir, dirname, 0755, 0, 0);
if (r < 0) {
bool ignore = r == -EROFS && try;
log_full_errno(ignore ? LOG_DEBUG : LOG_ERR, r,
@ -2592,10 +2605,14 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range
if (!p)
return log_oom();
_cleanup_free_ char *q = path_join(directory, p);
_cleanup_free_ char *q = path_join(persistent_path, p);
if (!q)
return log_oom();
_cleanup_free_ char *container_path = path_join(root_dir, p);
if (!container_path)
return log_oom();
if (path_is_mount_point(p) > 0) {
if (try)
return 0;
@ -2604,12 +2621,14 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range
"%s: already a mount point, refusing to use for journal", p);
}
if (path_is_mount_point(q) > 0) {
if (path_is_mount_point(container_path) > 0) {
if (try)
return 0;
return log_error_errno(SYNTHETIC_ERRNO(EEXIST),
"%s: already a mount point, refusing to use for journal", q);
return log_error_errno(
SYNTHETIC_ERRNO(EEXIST),
"%s: already a mount point, refusing to use for journal",
container_path);
}
r = readlink_and_make_absolute(p, &d);
@ -2617,9 +2636,9 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range
if (IN_SET(arg_link_journal, LINK_GUEST, LINK_AUTO) &&
path_equal(d, q)) {
r = userns_mkdir(directory, p, 0755, 0, 0);
r = userns_mkdir(root_dir, p, 0755, 0, 0);
if (r < 0)
log_warning_errno(r, "Failed to create directory %s: %m", q);
log_warning_errno(r, "Failed to create directory %s: %m", container_path);
return 0;
}
@ -2649,9 +2668,9 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range
return log_error_errno(errno, "Failed to symlink %s to %s: %m", q, p);
}
r = userns_mkdir(directory, p, 0755, 0, 0);
r = userns_mkdir(root_dir, p, 0755, 0, 0);
if (r < 0)
log_warning_errno(r, "Failed to create directory %s: %m", q);
log_warning_errno(r, "Failed to create directory %s: %m", container_path);
return 0;
}
@ -2671,25 +2690,25 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range
} else if (access(p, F_OK) < 0)
return 0;
if (dir_is_empty(q, /* ignore_hidden_or_backup= */ false) == 0)
log_warning("%s is not empty, proceeding anyway.", q);
if (dir_is_empty(container_path, /* ignore_hidden_or_backup= */ false) == 0)
log_warning("%s is not empty, proceeding anyway.", container_path);
r = userns_mkdir(directory, p, 0755, 0, 0);
r = userns_mkdir(root_dir, p, 0755, 0, 0);
if (r < 0)
return log_error_errno(r, "Failed to create %s: %m", q);
return log_error_errno(r, "Failed to create %s: %m", container_path);
return mount_custom(
directory,
root_dir,
&(CustomMount) {
.type = CUSTOM_MOUNT_BIND,
.options = (char*) (uid_is_valid(uid_shift) ? "rootidmap" : NULL),
.options = (char*) (uid_is_valid(chown_uid) ? "rootidmap" : NULL),
.source = p,
.destination = p,
.destination_uid = UID_INVALID,
},
/* n = */ 1,
uid_shift,
uid_range,
chown_uid,
chown_range,
arg_selinux_apifs_context,
MOUNT_NON_ROOT_ONLY);
}
@ -3861,6 +3880,7 @@ static DissectImageFlags determine_dissect_image_flags(void) {
static int outer_child(
Barrier *barrier,
const char *directory,
const char *persistent_path,
int mount_fd,
DissectedImage *dissected_image,
int fd_outer_socket,
@ -4267,7 +4287,7 @@ static int outer_child(
if (r < 0)
return r;
r = setup_journal(directory, chown_uid, chown_range);
r = setup_journal(directory, persistent_path, chown_uid, chown_range);
if (r < 0)
return r;
@ -5065,6 +5085,7 @@ static int load_oci_bundle(void) {
static int run_container(
const char *directory,
const char *persistent_path,
int mount_fd,
DissectedImage *dissected_image,
int userns_fd,
@ -5215,6 +5236,7 @@ static int run_container(
r = outer_child(&barrier,
directory,
persistent_path,
mount_fd,
dissected_image,
fd_outer_socket_pair[1],
@ -5927,6 +5949,7 @@ static int run(int argc, char *argv[]) {
_cleanup_(dissected_image_unrefp) DissectedImage *dissected_image = NULL;
_cleanup_(sd_netlink_unrefp) sd_netlink *nfnl = NULL;
_cleanup_(pidref_done) PidRef pid = PIDREF_NULL;
_cleanup_free_ char *persistent_path_storage = NULL;
log_setup();
@ -6388,6 +6411,47 @@ static int run(int argc, char *argv[]) {
arg_architecture = dissected_image_architecture(dissected_image);
}
/* Journal linking creates symlinks from the host's /var/log/journal/<uuid> to a persistent directory
* that survives container restarts, allowing journalctl -M to read container logs. But where that
* persistent directory is can be quite different depending on the mode:
*
* For --directory: The directory itself is the persistent location.
* For --image: Use /var/lib/machines/<machine-name> if it exists as a directory.
*
* Otherwise, we skip journal linking.
*/
const char *persistent_path;
if (arg_directory)
persistent_path = arg_directory;
else if (arg_image) {
persistent_path_storage = path_join("/var/lib/machines", arg_machine);
if (!persistent_path_storage) {
r = log_oom();
goto finish;
}
/* Validate that the persistent path exists and is actually a directory. We cannot create it
* ourselves as that would conflict with image management. */
struct stat st;
if (stat(persistent_path_storage, &st) < 0) {
if (errno == ENOENT)
log_debug("Persistent directory %s does not exist, skipping journal linking.",
persistent_path_storage);
else
log_warning_errno(
errno,
"Failed to access persistent directory %s, skipping journal linking: %m",
persistent_path_storage);
persistent_path = NULL;
} else if (!S_ISDIR(st.st_mode)) {
log_warning("%s exists but is not a directory, skipping journal linking.",
persistent_path_storage);
persistent_path = NULL;
} else
persistent_path = persistent_path_storage;
} else
assert_not_reached();
/* Create a temporary place to mount stuff. */
r = mkdtemp_malloc("/tmp/nspawn-root-XXXXXX", &rootdir);
if (r < 0) {
@ -6434,6 +6498,7 @@ static int run(int argc, char *argv[]) {
for (;;) {
r = run_container(
rootdir,
persistent_path,
mount_fd,
dissected_image,
userns_fd,

View File

@ -1533,4 +1533,130 @@ testcase_cap_net_bind_service() {
rm -fr "$root"
}
testcase_link_journal_directory() {
# Test that journal symlinks point to the persistent path
# for --directory.
local root machine_id journal_path symlink_target
root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-persist.XXX)"
create_dummy_container "$root"
machine_id="$(systemd-id128 new)"
echo "$machine_id" >"$root/etc/machine-id"
journal_path="/var/log/journal/$machine_id"
mkdir -p "$journal_path"
# Run container with journal linking
systemd-nspawn --register=no \
--directory="$root" \
--link-journal=try-guest \
bash -c "exit 0"
# Verify the symlink was created and is not broken
test -L "$journal_path"
test -e "$journal_path"
# Verify the symlink points to the persistent path (e.g., /var/lib/machines/...)
# NOT to a temporary /tmp/nspawn-root-* path
symlink_target="$(readlink -f "$journal_path")"
[[ "$symlink_target" == "$root"* ]]
[[ "$symlink_target" != /tmp/nspawn-root-* ]]
rm -f "$journal_path"
rm -fr "$root"
}
testcase_link_journal_image() {
# Test that journal symlinks point to the persistent path
# for --image.
local root machine_id journal_path symlink_target image mnt_dir
local machine_name="test-journal-image"
local persistent_dir="/var/lib/machines/$machine_name"
root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-image-root.XXX)"
image="$(mktemp /var/lib/machines/TEST-13-NSPAWN.journal-image.XXX.img)"
mnt_dir="$(mktemp -d)"
mkdir -p "$persistent_dir"
create_dummy_container "$root"
dd if=/dev/zero of="$image" bs=1M count=256 status=none
mkfs.ext4 -q "$image"
mount -o loop "$image" "$mnt_dir"
cp -r "$root"/* "$mnt_dir/"
machine_id="$(systemd-id128 new)"
echo "$machine_id" >"$mnt_dir/etc/machine-id"
umount "$mnt_dir"
journal_path="/var/log/journal/$machine_id"
mkdir -p "$journal_path"
# Run container with journal linking
systemd-nspawn --register=no \
--image="$image" \
--machine="$machine_name" \
--link-journal=try-guest \
bash -c "exit 0"
# Verify the symlink was created and points to the persistent directory.
# Note: We don't check that the target exists, as journald hasn't run in this test
# to create the directory structure - that's journald's responsibility, not nspawn's.
test -L "$journal_path"
symlink_target="$(readlink "$journal_path")"
[[ "$symlink_target" == "$persistent_dir"* ]]
rm -f "$journal_path"
rm -f "$image"
rm -fr "$root"
rm -fr "$mnt_dir"
rm -fr "$persistent_dir"
}
testcase_link_journal_ephemeral() {
# Test that journal symlinks are NOT created for --ephemeral.
local root machine_id journal_path
root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-ephemeral.XXX)"
create_dummy_container "$root"
machine_id="$(systemd-id128 new)"
echo "$machine_id" >"$root/etc/machine-id"
journal_path="/var/log/journal/$machine_id"
# Run ephemeral container with journal linking
systemd-nspawn --register=no \
--directory="$root" \
--ephemeral \
--link-journal=try-guest \
bash -c "exit 0"
# Verify the symlink was NOT created, as per existing logic
test ! -e "$journal_path"
rm -fr "$root"
}
testcase_link_journal_negative() {
# Test that no symlink is created when --link-journal is not used.
local root machine_id journal_path
root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-no-link.XXX)"
create_dummy_container "$root"
machine_id="$(systemd-id128 new)"
echo "$machine_id" >"$root/etc/machine-id"
journal_path="/var/log/journal/$machine_id"
# Run container WITHOUT journal linking
systemd-nspawn --register=no \
--directory="$root" \
bash -c "exit 0"
# Verify the symlink was NOT created
test ! -e "$journal_path"
rm -fr "$root"
}
run_testcases