mirror of
https://github.com/morgan9e/systemd
synced 2026-04-14 00:14:32 +09:00
nspawn: Prevent invalid UIDs propagating in bind mounts (#39729)
Commit 88fce09026 modified the
mount_bind() function, causing it to perform arithmetic on the uid_shift
parameter. However, it performs this arithmetic even when uid_shift was
UID_INVALID, which was not intended. This typically occurred when
mount_custom() was called for a simple bind mount without user
namespaces (and thus no rootidmap mount option).
This arithmetic (e.g., uid_shift + m->destination_uid) then wraps
around, resulting in the invalid ID 4294967295 ((uid_t)-1).
This bug manifests for users running systemd-nspawn with
--link-journal=host and --volatile=yes (but without --private-users),
causing systemd-tmpfiles to fail.
Make mount_bind() robust by checking if uid_shift is valid before using
it in arithmetic. If it is UID_INVALID, it defaults to a shift of 0 for
the ownership calculation, restoring correct behavior for plain bind
mounts while preserving the intended logic for ID-mapped mounts.
Fixes: #39714
This commit is contained in:
@@ -814,9 +814,20 @@ static int mount_bind(const char *dest, CustomMount *m, uid_t uid_shift, uid_t u
|
||||
return r;
|
||||
}
|
||||
|
||||
/* ID remapping cannot be done if user namespaces are not in use (uid_shift is UID_INVALID).
|
||||
* Fail if idmapping was explicitly requested in this state. Otherwise, treat UID_INVALID
|
||||
* as 0 for ownership operations. */
|
||||
if (idmapping != REMOUNT_IDMAPPING_NONE && !uid_is_valid(uid_shift))
|
||||
return log_error_errno(
|
||||
SYNTHETIC_ERRNO(EINVAL),
|
||||
"ID remapping requested for %s, but user namespacing is not enabled.",
|
||||
m->source);
|
||||
|
||||
uid_t chown_uid = uid_is_valid(uid_shift) ? uid_shift : 0;
|
||||
|
||||
/* If this is a bind mount from a temporary sources change ownership of the source to the container's
|
||||
* root UID. Otherwise it would always show up as "nobody" if user namespacing is used. */
|
||||
if (m->rm_rf_tmpdir && chown(m->source, uid_shift, uid_shift) < 0)
|
||||
if (m->rm_rf_tmpdir && chown(m->source, chown_uid, chown_uid) < 0)
|
||||
return log_error_errno(errno, "Failed to chown %s: %m", m->source);
|
||||
|
||||
/* UID/GIDs of idmapped mounts are always resolved in the caller's user namespace. In other
|
||||
@@ -850,7 +861,7 @@ static int mount_bind(const char *dest, CustomMount *m, uid_t uid_shift, uid_t u
|
||||
if (stat(where, &dest_st) < 0)
|
||||
return log_error_errno(errno, "Failed to stat %s: %m", where);
|
||||
|
||||
dest_uid = uid_is_valid(m->destination_uid) ? uid_shift + m->destination_uid : dest_st.st_uid;
|
||||
dest_uid = uid_is_valid(m->destination_uid) ? chown_uid + m->destination_uid : dest_st.st_uid;
|
||||
|
||||
if (S_ISDIR(source_st.st_mode) && !S_ISDIR(dest_st.st_mode))
|
||||
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
|
||||
@@ -863,7 +874,7 @@ static int mount_bind(const char *dest, CustomMount *m, uid_t uid_shift, uid_t u
|
||||
m->source, where);
|
||||
|
||||
} else { /* Path doesn't exist yet? */
|
||||
r = mkdir_parents_safe_label(dest, where, 0755, uid_shift, uid_shift, MKDIR_IGNORE_EXISTING);
|
||||
r = mkdir_parents_safe_label(dest, where, 0755, chown_uid, chown_uid, MKDIR_IGNORE_EXISTING);
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to make parents of %s: %m", where);
|
||||
|
||||
@@ -878,10 +889,10 @@ static int mount_bind(const char *dest, CustomMount *m, uid_t uid_shift, uid_t u
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to create mount point %s: %m", where);
|
||||
|
||||
if (chown(where, uid_shift, uid_shift) < 0)
|
||||
if (chown(where, chown_uid, chown_uid) < 0)
|
||||
return log_error_errno(errno, "Failed to chown %s: %m", where);
|
||||
|
||||
dest_uid = uid_shift + (uid_is_valid(m->destination_uid) ? m->destination_uid : 0);
|
||||
dest_uid = chown_uid + (uid_is_valid(m->destination_uid) ? m->destination_uid : 0);
|
||||
}
|
||||
|
||||
if (move_mount(fd_clone, "", AT_FDCWD, where, MOVE_MOUNT_F_EMPTY_PATH) < 0)
|
||||
|
||||
@@ -1477,6 +1477,44 @@ testcase_link_journal_host() {
|
||||
rm -fr "$root" "$hoge"
|
||||
}
|
||||
|
||||
testcase_volatile_link_journal_no_userns() {
|
||||
local root machine_id journal_dir acl_output
|
||||
|
||||
root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.volatile-journal.XXX)"
|
||||
create_dummy_container "$root"
|
||||
|
||||
machine_id="$(systemd-id128 new)"
|
||||
echo "$machine_id" >"$root/etc/machine-id"
|
||||
|
||||
journal_dir="/var/log/journal/$machine_id"
|
||||
mkdir -p "$journal_dir"
|
||||
chown root:root "$journal_dir"
|
||||
|
||||
systemd-nspawn --register=no \
|
||||
--directory="$root" \
|
||||
--boot \
|
||||
--volatile=yes \
|
||||
--link-journal=host \
|
||||
systemd.unit=systemd-tmpfiles-setup.service
|
||||
|
||||
local gid
|
||||
gid="$(stat -c '%g' "$journal_dir")"
|
||||
|
||||
# Ensure GID is not 4294967295 (GID_INVALID)
|
||||
[[ "$gid" != "4294967295" ]]
|
||||
|
||||
# Ensure the directory is owned by a valid user (root or systemd-journal
|
||||
# group). The GID should be either 0 (root) or the systemd-journal GID, not
|
||||
# some bombastically large number
|
||||
[[ "$gid" -lt 65535 ]]
|
||||
|
||||
# Ensure the invalid GID doesn't appear in ACLs
|
||||
acl_output="$(getfacl "$journal_dir" || true)"
|
||||
grep -q "4294967295" <<< "$acl_output" && exit 1
|
||||
|
||||
rm -fr "$root" "$journal_dir"
|
||||
}
|
||||
|
||||
testcase_cap_net_bind_service() {
|
||||
local root
|
||||
|
||||
|
||||
Reference in New Issue
Block a user