From 9842905ede9c7fdc541724ee5c6db7d46a47405d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Mar 2021 22:03:00 +0100 Subject: [PATCH 1/3] dissect-image: clean up meaning of DISSECT_IMAGE_MKDIR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously handling of DISSECT_IMAGE_MKDIR was pretty weird and broken: it would control both if we create the top-level mount point when mounting an image, and the inner mount points for images that consist of multiple file systems. However, the latter is redundant, since 1f0f82f1311e4c52152b8e2b6f266258709c137d does this too, a few lines further up – unconditionally! Hence, let's make the meaning of DISSECT_IMAGE_MKDIR more strict: it shall be only about the top-level mount point, not about the inner ones (where we'll continue to create what is missing alwayway). Having a separate flag for the top-level mount point is relevant, since the mount point dir created by it will remain on the host fs – unlike the directories we create inside the image, which will stay within the image. This slightly change of meaning is actually inline with what the flag is actually used for and documented in systemd-dissect. --- src/shared/dissect-image.c | 24 ++++++++++++------------ src/shared/dissect-image.h | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 32fff8165b..d905308762 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1345,7 +1345,7 @@ static int mount_partition( if (directory) { if (!FLAGS_SET(flags, DISSECT_IMAGE_READ_ONLY)) { - /* Automatically create missing mount points, if necessary. */ + /* Automatically create missing mount points inside the image, if necessary. */ r = mkdir_p_root(where, directory, uid_shift, (gid_t) uid_shift, 0755); if (r < 0) return r; @@ -1356,8 +1356,18 @@ static int mount_partition( return r; p = chased; - } else + } else { + /* Create top-level mount if missing – but only if this is asked for. This won't modify the + * image (as the branch above does) but the host hierarchy, and the created directory might + * survive our mount in the host hierarchy hence. */ + if (FLAGS_SET(flags, DISSECT_IMAGE_MKDIR)) { + r = mkdir_p(where, 0755); + if (r < 0) + return r; + } + p = where; + } /* If requested, turn on discard support. */ if (fstype_can_discard(fstype) && @@ -1382,12 +1392,6 @@ static int mount_partition( if (!strextend_with_separator(&options, ",", m->mount_options)) return -ENOMEM; - if (FLAGS_SET(flags, DISSECT_IMAGE_MKDIR)) { - r = mkdir_p(p, 0755); - if (r < 0) - return r; - } - r = mount_nofollow_verbose(LOG_DEBUG, node, p, fstype, MS_NODEV|(rw ? 0 : MS_RDONLY), options); if (r < 0) return r; @@ -1420,10 +1424,6 @@ int dissected_image_mount(DissectedImage *m, const char *where, uid_t uid_shift, return r; } - /* Mask DISSECT_IMAGE_MKDIR for all subdirs: the idea is that only the top-level mount point is - * created if needed, but the image itself not modified. */ - flags &= ~DISSECT_IMAGE_MKDIR; - if ((flags & DISSECT_IMAGE_MOUNT_NON_ROOT_ONLY) == 0) { /* For us mounting root always means mounting /usr as well */ r = mount_partition(m->partitions + PARTITION_USR, where, "/usr", uid_shift, flags); diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h index 77e7c80c20..ddadda1c0c 100644 --- a/src/shared/dissect-image.h +++ b/src/shared/dissect-image.h @@ -86,7 +86,7 @@ typedef enum DissectImageFlags { DISSECT_IMAGE_FSCK = 1 << 11, /* File system check the partition before mounting (no effect when combined with DISSECT_IMAGE_READ_ONLY) */ DISSECT_IMAGE_NO_PARTITION_TABLE = 1 << 12, /* Only recognize single file system images */ DISSECT_IMAGE_VERITY_SHARE = 1 << 13, /* When activating a verity device, reuse existing one if already open */ - DISSECT_IMAGE_MKDIR = 1 << 14, /* Make directory to mount right before mounting, if missing */ + DISSECT_IMAGE_MKDIR = 1 << 14, /* Make top-level directory to mount right before mounting, if missing */ } DissectImageFlags; struct DissectedImage { From 334eb5b0999d51efbc38b775fca92a2556c85830 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 11 Mar 2021 11:44:03 +0100 Subject: [PATCH 2/3] dissect-image: fix volatile images This makes sure nspawn's --volatile=yes switch works again: there we have a read-only image that is overmounted by a tmpfs (with the exception of /usr). This we need to mkdir all mount points even though the image is read-only. Hence, let's drop the optimizatio of avoiding mkdir() on images that are read-only, it's wrong and misleading here, since the image itself might be read-only but our mounts are not. --- src/shared/dissect-image.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index d905308762..6f9a457614 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1344,12 +1344,10 @@ static int mount_partition( } if (directory) { - if (!FLAGS_SET(flags, DISSECT_IMAGE_READ_ONLY)) { - /* Automatically create missing mount points inside the image, if necessary. */ - r = mkdir_p_root(where, directory, uid_shift, (gid_t) uid_shift, 0755); - if (r < 0) - return r; - } + /* Automatically create missing mount points inside the image, if necessary. */ + r = mkdir_p_root(where, directory, uid_shift, (gid_t) uid_shift, 0755); + if (r < 0 && r != -EROFS) + return r; r = chase_symlinks(directory, where, CHASE_PREFIX_ROOT, &chased, NULL); if (r < 0) From 38db55ab0c88994a80d79313294c95b236645f1b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Mar 2021 22:01:41 +0100 Subject: [PATCH 3/3] dissect: fix trivial typo --- src/dissect/dissect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index b9adbb6f7f..cb96a57abe 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -631,7 +631,7 @@ static int action_copy(DissectedImage *m, LoopDevice *d) { if (r < 0) return log_error_errno(r, "Failed to copy bytes from %s in mage '%s' to stdout: %m", arg_source, arg_image); - /* When we copy to stdou we don't copy any attributes (i.e. no access mode, no ownership, no xattr, no times) */ + /* When we copy to stdout we don't copy any attributes (i.e. no access mode, no ownership, no xattr, no times) */ return 0; }