From a1a40297dbfa5bcd926d1a19320deb73c033c6f5 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 4 Jun 2024 16:00:03 +0100 Subject: [PATCH] core: deduplicate identical dm-verity ExtensionImages= It turns out OverlayFS doesn't handle gracefully when the same source is specified multiple times in lowerdir= and it fails with ELOOP: Failed to mount overlay (type overlay) on /run/systemd/mount-rootfs/opt (MS_RDONLY "lowerdir=/run/systemd/unit-extensions/1/opt:/run/systemd/unit-extensions/0/opt:/run/systemd/mount-rootfs/opt"): Too many levels of symbolic links This happens even if we mount each image in a different internal mount path, as OverlayFS will resolve it and look for the backing device, which will be the same device mapper entity, and return a hard error. This error does not appear if dm-verity is not used, so it is very confusing for users, and unnecessary. When mounting ExtensionImages, check if an image is dm-veritied, and drop duplicates if the root hashes match, to avoid this user-unfriendly hard error. --- src/core/namespace.c | 47 ++++++++++++++++++++++++++- src/shared/dissect-image.c | 31 +++++++++++------- src/shared/dissect-image.h | 2 +- src/shared/mount-util.c | 2 ++ test/TEST-50-DISSECT/test.sh | 1 + test/units/TEST-50-DISSECT.dissect.sh | 16 +++++++++ 6 files changed, 85 insertions(+), 14 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 6d3cadf05cb..57518ad7ff8 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -112,6 +112,7 @@ typedef struct MountEntry { unsigned n_followed; LIST_HEAD(MountOptions, image_options_const); char **overlay_layers; + VeritySettings verity; } MountEntry; typedef struct MountList { @@ -343,6 +344,7 @@ static void mount_entry_done(MountEntry *p) { p->source_malloc = mfree(p->source_malloc); p->options_malloc = mfree(p->options_malloc); p->overlay_layers = strv_free(p->overlay_layers); + verity_settings_done(&p->verity); } static void mount_list_done(MountList *ml) { @@ -505,6 +507,7 @@ static int append_extensions( /* First, prepare a mount for each image, but these won't be visible to the unit, instead * they will be mounted in our propagate directory, and used as a source for the overlay. */ for (size_t i = 0; i < n_mount_images; i++) { + _cleanup_(verity_settings_done) VeritySettings verity = VERITY_SETTINGS_DEFAULT; _cleanup_(pick_result_done) PickResult result = PICK_RESULT_NULL; _cleanup_free_ char *mount_point = NULL; const MountImage *m = mount_images + i; @@ -523,6 +526,10 @@ static int append_extensions( "No matching entry in .v/ directory %s found.", m->source); + r = verity_settings_load(&verity, result.path, /* root_hash_path= */ NULL, /* root_hash_sig_path= */ NULL); + if (r < 0) + return log_debug_errno(r, "Failed to check verity root hash of %s: %m", result.path); + if (asprintf(&mount_point, "%s/unit-extensions/%zu", private_namespace_dir, i) < 0) return -ENOMEM; @@ -547,6 +554,7 @@ static int append_extensions( .source_malloc = TAKE_PTR(result.path), .mode = MOUNT_EXTENSION_IMAGE, .has_prefix = true, + .verity = TAKE_GENERIC(verity, VeritySettings, VERITY_SETTINGS_DEFAULT), }; } @@ -787,6 +795,36 @@ static int prefix_where_needed(MountList *ml, const char *root_directory) { return 0; } +static bool verity_has_later_duplicates(MountList *ml, const MountEntry *needle) { + + assert(ml); + assert(needle); + assert(needle >= ml->mounts && needle < ml->mounts + ml->n_mounts); + assert(needle->mode == MOUNT_EXTENSION_IMAGE); + + if (needle->verity.root_hash_size == 0) + return false; + + /* Overlayfs rejects supplying the same directory inode twice as determined by filesystem UUID and + * file handle in lowerdir=, even if they are mounted on different paths, as it resolves each mount + * to its source filesystem, so drop duplicates, and keep the last one. This only covers non-DDI + * verity images. Note that the list is ordered, so we only check for the reminder of the list for + * each item, rather than the full list from the beginning, as any earlier duplicates will have + * already been pruned. */ + + for (const MountEntry *m = needle + 1; m < ml->mounts + ml->n_mounts; m++) { + if (m->mode != MOUNT_EXTENSION_IMAGE) + continue; + if (memcmp_nn(m->verity.root_hash, + m->verity.root_hash_size, + needle->verity.root_hash, + needle->verity.root_hash_size) == 0) + return true; + } + + return false; +} + static void drop_duplicates(MountList *ml) { MountEntry *f, *t, *previous; @@ -810,6 +848,12 @@ static void drop_duplicates(MountList *ml) { continue; } + if (f->mode == MOUNT_EXTENSION_IMAGE && verity_has_later_duplicates(ml, f)) { + log_debug("Skipping duplicate extension image %s", mount_entry_source(f)); + mount_entry_done(f); + continue; + } + *t = *f; previous = t; t++; @@ -1347,7 +1391,7 @@ static int mount_mqueuefs(const MountEntry *m) { } static int mount_image( - const MountEntry *m, + MountEntry *m, const char *root_directory, const ImagePolicy *image_policy) { @@ -1387,6 +1431,7 @@ static int mount_image( host_os_release_sysext_level, host_os_release_confext_level, /* required_sysext_scope= */ NULL, + &m->verity, /* ret_image= */ NULL); if (r == -ENOENT && m->ignore) return 0; diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 0d23e519310..1a3ed0cf7f4 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -4034,11 +4034,12 @@ int verity_dissect_and_mount( const char *required_host_os_release_sysext_level, const char *required_host_os_release_confext_level, const char *required_sysext_scope, + VeritySettings *verity, DissectedImage **ret_image) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; _cleanup_(dissected_image_unrefp) DissectedImage *dissected_image = NULL; - _cleanup_(verity_settings_done) VeritySettings verity = VERITY_SETTINGS_DEFAULT; + _cleanup_(verity_settings_done) VeritySettings local_verity = VERITY_SETTINGS_DEFAULT; DissectImageFlags dissect_image_flags; bool relax_extension_release_check; int r; @@ -4050,13 +4051,19 @@ int verity_dissect_and_mount( relax_extension_release_check = mount_options_relax_extension_release_checks(options); - /* We might get an FD for the image, but we use the original path to look for the dm-verity files */ - r = verity_settings_load(&verity, src, NULL, NULL); - if (r < 0) - return log_debug_errno(r, "Failed to load root hash: %m"); + /* We might get an FD for the image, but we use the original path to look for the dm-verity files. + * The caller might also give us a pre-loaded VeritySettings, in which case we just use it. It will + * also be extended, as dissected_image_load_verity_sig_partition() is invoked. */ + if (!verity) { + r = verity_settings_load(&local_verity, src, NULL, NULL); + if (r < 0) + return log_debug_errno(r, "Failed to load root hash: %m"); + + verity = &local_verity; + } dissect_image_flags = - (verity.data_path ? DISSECT_IMAGE_NO_PARTITION_TABLE : 0) | + (verity->data_path ? DISSECT_IMAGE_NO_PARTITION_TABLE : 0) | (relax_extension_release_check ? DISSECT_IMAGE_RELAX_EXTENSION_CHECK : 0) | DISSECT_IMAGE_ADD_PARTITION_DEVICES | DISSECT_IMAGE_PIN_PARTITION_DEVICES | @@ -4068,7 +4075,7 @@ int verity_dissect_and_mount( src_fd >= 0 ? FORMAT_PROC_FD_PATH(src_fd) : src, /* open_flags= */ -1, /* sector_size= */ UINT32_MAX, - verity.data_path ? 0 : LO_FLAGS_PARTSCAN, + verity->data_path ? 0 : LO_FLAGS_PARTSCAN, LOCK_SH, &loop_device); if (r < 0) @@ -4076,16 +4083,16 @@ int verity_dissect_and_mount( r = dissect_loop_device( loop_device, - &verity, + verity, options, image_policy, dissect_image_flags, &dissected_image); /* No partition table? Might be a single-filesystem image, try again */ - if (!verity.data_path && r == -ENOPKG) + if (!verity->data_path && r == -ENOPKG) r = dissect_loop_device( loop_device, - &verity, + verity, options, image_policy, dissect_image_flags | DISSECT_IMAGE_NO_PARTITION_TABLE, @@ -4093,14 +4100,14 @@ int verity_dissect_and_mount( if (r < 0) return log_debug_errno(r, "Failed to dissect image: %m"); - r = dissected_image_load_verity_sig_partition(dissected_image, loop_device->fd, &verity); + r = dissected_image_load_verity_sig_partition(dissected_image, loop_device->fd, verity); if (r < 0) return r; r = dissected_image_decrypt( dissected_image, NULL, - &verity, + verity, dissect_image_flags); if (r < 0) return log_debug_errno(r, "Failed to decrypt dissected image: %m"); diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h index 02aa2d5734b..2c118b97ef5 100644 --- a/src/shared/dissect-image.h +++ b/src/shared/dissect-image.h @@ -226,7 +226,7 @@ bool dissected_image_verity_sig_ready(const DissectedImage *image, PartitionDesi int mount_image_privately_interactively(const char *path, const ImagePolicy *image_policy, DissectImageFlags flags, char **ret_directory, int *ret_dir_fd, LoopDevice **ret_loop_device); -int verity_dissect_and_mount(int src_fd, const char *src, const char *dest, const MountOptions *options, const ImagePolicy *image_policy, const char *required_host_os_release_id, const char *required_host_os_release_version_id, const char *required_host_os_release_sysext_level, const char *required_host_os_release_confext_level, const char *required_sysext_scope, DissectedImage **ret_image); +int verity_dissect_and_mount(int src_fd, const char *src, const char *dest, const MountOptions *options, const ImagePolicy *image_policy, const char *required_host_os_release_id, const char *required_host_os_release_version_id, const char *required_host_os_release_sysext_level, const char *required_host_os_release_confext_level, const char *required_sysext_scope, VeritySettings *verity, DissectedImage **ret_image); int dissect_fstype_ok(const char *fstype); diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index 4ddfb9a82ac..a1f4bee77e1 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -941,6 +941,7 @@ static int mount_in_namespace_legacy( /* required_host_os_release_sysext_level= */ NULL, /* required_host_os_release_confext_level= */ NULL, /* required_sysext_scope= */ NULL, + /* verity= */ NULL, /* ret_image= */ NULL); else r = mount_follow_verbose(LOG_DEBUG, FORMAT_PROC_FD_PATH(chased_src_fd), mount_tmp, NULL, MS_BIND, NULL); @@ -1166,6 +1167,7 @@ static int mount_in_namespace( /* required_host_os_release_sysext_level= */ NULL, /* required_host_os_release_confext_level= */ NULL, /* required_sysext_scope= */ NULL, + /* verity= */ NULL, &img); if (r < 0) return log_debug_errno( diff --git a/test/TEST-50-DISSECT/test.sh b/test/TEST-50-DISSECT/test.sh index a0cd677f075..de20ac4681b 100755 --- a/test/TEST-50-DISSECT/test.sh +++ b/test/TEST-50-DISSECT/test.sh @@ -37,6 +37,7 @@ test_append_files() { inst_binary mksquashfs inst_binary unsquashfs inst_binary pkcheck + inst_binary veritysetup install_verity_minimal } diff --git a/test/units/TEST-50-DISSECT.dissect.sh b/test/units/TEST-50-DISSECT.dissect.sh index 563206ce7e4..9a05899cabc 100755 --- a/test/units/TEST-50-DISSECT.dissect.sh +++ b/test/units/TEST-50-DISSECT.dissect.sh @@ -389,6 +389,22 @@ systemd-run -P \ --property RootImage="$MINIMAL_IMAGE.raw" \ "${BIND_LOG_SOCKETS[@]}" \ cat /etc/systemd/system/some_file | grep -q -F "MARKER_CONFEXT_123" + +# Check that two identical verity images at different paths do not fail with -ELOOP from OverlayFS +mkdir -p /tmp/loop +cp /tmp/app0.raw /tmp/loop/app0.raw +veritysetup format /tmp/loop/app0.raw /tmp/loop/app0.verity --root-hash-file /tmp/loop/app0.roothash +cp /tmp/loop/app0.raw /tmp/loop/app0_copy.raw +cp /tmp/loop/app0.verity /tmp/loop/app0_copy.verity +cp /tmp/loop/app0.roothash /tmp/loop/app0_copy.roothash +systemd-run -P \ + --property ExtensionImages=/tmp/loop/app0.raw \ + --property ExtensionImages=/tmp/loop/app0_copy.raw \ + --property RootImage="$MINIMAL_IMAGE.raw" \ + "${BIND_LOG_SOCKETS[@]}" \ + cat /opt/script0.sh | grep -q -F "extension-release.app0" +rm -rf /tmp/loop/ + # Check that using a symlink to NAME-VERSION.raw works as long as the symlink has the correct name NAME.raw mkdir -p /tmp/symlink-test/ cp /tmp/app-nodistro.raw /tmp/symlink-test/app-nodistro-v1.raw