Skip to content

Commit 2fa017f

Browse files
LukeShuLukeShu
committed
nspawn: Simplify tmpfs_patch_options() usage, and trickle that up
One of the things that tmpfs_patch_options does is take an (optional) UID, and insert "uid=${UID},gid=${UID}" into the options string. So we need a uid_t argument, and a way of telling if we should use it. Fortunately, that is built in to the uid_t type by having UID_INVALID as a possible value. So this is really a feature that requires one argument. Yet, it is somehow taking 4! That is absurd. Simplify it to only take one argument, and have that trickle all the way up to mount_all()'s usage. Now, in may of the uses, the argument becomes uid_shift == 0 ? UID_INVALID : uid_shift because it used to treat uid_shift=0 as invalid unless the patch_ids flag was also set. This keeps the behavior the same. Note that in all cases where it is invoked, if !use_userns (sometimes called !userns), then uid_shift is 0; we don't have to add any checks for that. That said, I'm pretty sure that "uid=0" and not setting "uid=" are the same, but Christian Brauner seemed to not think so when implementing the cgns support. systemd#3589
1 parent 9c0fad5 commit 2fa017f

File tree

3 files changed

+10
-19
lines changed

3 files changed

+10
-19
lines changed

‎src/nspawn/nspawn-mount.c‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -329,17 +329,13 @@ int overlay_mount_parse(CustomMount **l, size_t *n, const char *s, bool read_onl
329329

330330
static int tmpfs_patch_options(
331331
const char *options,
332-
bool userns,
333-
uid_t uid_shift, uid_t uid_range,
334-
bool patch_ids,
332+
uid_t uid_shift,
335333
const char *selinux_apifs_context,
336334
char **ret) {
337335

338336
char *buf = NULL;
339337

340-
if ((userns && uid_shift != 0) || patch_ids) {
341-
assert(uid_shift != UID_INVALID);
342-
338+
if (uid_shift != UID_INVALID) {
343339
if (asprintf(&buf, "%s%suid=" UID_FMT ",gid=" UID_FMT,
344340
strempty(options), options ? "," : "",
345341
uid_shift, uid_shift) < 0)
@@ -497,7 +493,7 @@ static int mkdir_userns_p(const char *prefix, const char *path, mode_t mode, uid
497493

498494
int mount_all(const char *dest,
499495
MountSettingsMask mount_settings,
500-
uid_t uid_shift, uid_t uid_range,
496+
uid_t uid_shift,
501497
const char *selinux_apifs_context) {
502498

503499
#define PROC_INACCESSIBLE(path) \
@@ -646,10 +642,7 @@ int mount_all(const char *dest,
646642

647643
o = mount_table[k].options;
648644
if (streq_ptr(mount_table[k].type, "tmpfs")) {
649-
if (in_userns)
650-
r = tmpfs_patch_options(o, use_userns, 0, uid_range, true, selinux_apifs_context, &options);
651-
else
652-
r = tmpfs_patch_options(o, use_userns, uid_shift, uid_range, false, selinux_apifs_context, &options);
645+
r = tmpfs_patch_options(o, in_userns ? 0 : uid_shift, selinux_apifs_context, &options);
653646
if (r < 0)
654647
return log_oom();
655648
if (r > 0)
@@ -752,7 +745,7 @@ static int mount_tmpfs(
752745
return log_error_errno(r, "Creating mount point for tmpfs %s failed: %m", where);
753746
}
754747

755-
r = tmpfs_patch_options(m->options, userns, uid_shift, uid_range, false, selinux_apifs_context, &buf);
748+
r = tmpfs_patch_options(m->options, uid_shift == 0 ? UID_INVALID : uid_shift, selinux_apifs_context, &buf);
756749
if (r < 0)
757750
return log_oom();
758751
options = r > 0 ? buf : m->options;
@@ -985,7 +978,7 @@ static int mount_legacy_cgns_supported(
985978
* uid/gid as seen from e.g. /proc/1/mountinfo. So we simply
986979
* pass uid 0 and not uid_shift to tmpfs_patch_options().
987980
*/
988-
r = tmpfs_patch_options("mode=755", userns, 0, uid_range, true, selinux_apifs_context, &options);
981+
r = tmpfs_patch_options("mode=755", 0, selinux_apifs_context, &options);
989982
if (r < 0)
990983
return log_oom();
991984

@@ -1087,7 +1080,7 @@ static int mount_legacy_cgns_unsupported(
10871080
if (r == 0) {
10881081
_cleanup_free_ char *options = NULL;
10891082

1090-
r = tmpfs_patch_options("mode=755", userns, uid_shift, uid_range, false, selinux_apifs_context, &options);
1083+
r = tmpfs_patch_options("mode=755", uid_shift == 0 ? UID_INVALID : uid_shift, selinux_apifs_context, &options);
10911084
if (r < 0)
10921085
return log_oom();
10931086

@@ -1298,7 +1291,7 @@ int setup_volatile_state(
12981291
return log_error_errno(errno, "Failed to create %s: %m", directory);
12991292

13001293
options = "mode=755";
1301-
r = tmpfs_patch_options(options, userns, uid_shift, uid_range, false, selinux_apifs_context, &buf);
1294+
r = tmpfs_patch_options(options, uid_shift == 0 ? UID_INVALID : uid_shift, selinux_apifs_context, &buf);
13021295
if (r < 0)
13031296
return log_oom();
13041297
if (r > 0)
@@ -1331,7 +1324,7 @@ int setup_volatile(
13311324
return log_error_errno(errno, "Failed to create temporary directory: %m");
13321325

13331326
options = "mode=755";
1334-
r = tmpfs_patch_options(options, userns, uid_shift, uid_range, false, selinux_apifs_context, &buf);
1327+
r = tmpfs_patch_options(options, uid_shift == 0 ? UID_INVALID : uid_shift, selinux_apifs_context, &buf);
13351328
if (r < 0)
13361329
return log_oom();
13371330
if (r > 0)

‎src/nspawn/nspawn-mount.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ int bind_mount_parse(CustomMount **l, size_t *n, const char *s, bool read_only);
4343
int tmpfs_mount_parse(CustomMount **l, size_t *n, const char *s);
4444
int overlay_mount_parse(CustomMount **l, size_t *n, const char *s, bool read_only);
4545

46-
int mount_all(const char *dest, MountSettingsMask mount_settings, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context);
46+
int mount_all(const char *dest, MountSettingsMask mount_settings, uid_t uid_shift, const char *selinux_apifs_context);
4747
int mount_sysfs(const char *dest, MountSettingsMask mount_settings);
4848

4949
int mount_cgroups(const char *dest, CGroupUnified unified_requested, bool userns, uid_t uid_shift, uid_t uid_range, const char *selinux_apifs_context, bool use_cgns);

‎src/nspawn/nspawn.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2558,7 +2558,6 @@ static int inner_child(
25582558
r = mount_all(NULL,
25592559
arg_mount_settings | MOUNT_IN_USERNS,
25602560
arg_uid_shift,
2561-
arg_uid_range,
25622561
arg_selinux_apifs_context);
25632562
if (r < 0)
25642563
return r;
@@ -2990,7 +2989,6 @@ static int outer_child(
29902989
r = mount_all(directory,
29912990
arg_mount_settings,
29922991
arg_uid_shift,
2993-
arg_uid_range,
29942992
arg_selinux_apifs_context);
29952993
if (r < 0)
29962994
return r;

0 commit comments

Comments
 (0)