From 2c07d314b2d9309c4d91a966a01a8d1b76898c1d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Sep 2023 15:37:58 +0200 Subject: [PATCH 01/15] fileio: revamp search_and_fopen() Let's modernize and clean up search_and_fopen a bit: let's add support for regular open() (instead of fopen()), as well as access() (if caller just wants to check if a file exists without opening it. This unifies much of the code involved, which previously was duplicated in search_and_fopen() and search_and_fopen_nulstr() --- src/basic/fileio.c | 170 +++++++++++++++++++++++++++-------------- src/basic/fileio.h | 8 +- src/test/test-fileio.c | 23 ++++++ 3 files changed, 141 insertions(+), 60 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index c41db65eb1..527f53daee 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1076,117 +1076,171 @@ int fdopen_independent(int fd, const char *mode, FILE **ret) { return 0; } -static int search_and_fopen_internal( +static int search_and_open_internal( const char *path, - const char *mode, + int mode, /* if ret_fd is NULL this is an [FRWX]_OK mode for access(), otherwise an open mode for open() */ const char *root, char **search, - FILE **ret, + int *ret_fd, char **ret_path) { + int r; + + assert(!ret_fd || !FLAGS_SET(mode, O_CREAT)); /* We don't support O_CREAT for this */ assert(path); - assert(mode); - assert(ret); + + if (path_is_absolute(path)) { + _cleanup_close_ int fd = -EBADF; + + if (ret_fd) + /* We only specify 0777 here to appease static analyzers, it's never used since we + * don't support O_CREAT here */ + r = fd = RET_NERRNO(open(path, mode, 0777)); + else + r = RET_NERRNO(access(path, mode)); + if (r < 0) + return r; + + if (ret_path) { + r = path_simplify_alloc(path, ret_path); + if (r < 0) + return r; + } + + if (ret_fd) + *ret_fd = TAKE_FD(fd); + + return 0; + } if (!path_strv_resolve_uniq(search, root)) return -ENOMEM; STRV_FOREACH(i, search) { + _cleanup_close_ int fd = -EBADF; _cleanup_free_ char *p = NULL; - FILE *f; p = path_join(root, *i, path); if (!p) return -ENOMEM; - f = fopen(p, mode); - if (f) { + if (ret_fd) + /* as above, 0777 is static analyzer appeasement */ + r = fd = RET_NERRNO(open(p, mode, 0777)); + else + r = RET_NERRNO(access(p, F_OK)); + if (r >= 0) { if (ret_path) *ret_path = path_simplify(TAKE_PTR(p)); - *ret = f; + if (ret_fd) + *ret_fd = TAKE_FD(fd); + return 0; } - - if (errno != ENOENT) - return -errno; + if (r != -ENOENT) + return r; } return -ENOENT; } -int search_and_fopen( - const char *filename, - const char *mode, +int search_and_open( + const char *path, + int mode, const char *root, - const char **search, - FILE **ret, + char **search, + int *ret_fd, char **ret_path) { _cleanup_strv_free_ char **copy = NULL; - int r; - assert(filename); - assert(mode); - assert(ret); - - if (path_is_absolute(filename)) { - _cleanup_fclose_ FILE *f = NULL; - - f = fopen(filename, mode); - if (!f) - return -errno; - - if (ret_path) { - r = path_simplify_alloc(filename, ret_path); - if (r < 0) - return r; - } - - *ret = TAKE_PTR(f); - return 0; - } + assert(path); copy = strv_copy((char**) search); if (!copy) return -ENOMEM; - return search_and_fopen_internal(filename, mode, root, copy, ret, ret_path); + return search_and_open_internal(path, mode, root, copy, ret_fd, ret_path); } -int search_and_fopen_nulstr( - const char *filename, +static int search_and_fopen_internal( + const char *path, const char *mode, const char *root, - const char *search, - FILE **ret, + char **search, + FILE **ret_file, char **ret_path) { - _cleanup_strv_free_ char **s = NULL; + _cleanup_free_ char *found_path = NULL; + _cleanup_close_ int fd = -EBADF; int r; - if (path_is_absolute(filename)) { - _cleanup_fclose_ FILE *f = NULL; + assert(path); + assert(mode || !ret_file); - f = fopen(filename, mode); + r = search_and_open( + path, + mode ? fopen_mode_to_flags(mode) : 0, + root, + search, + ret_file ? &fd : NULL, + ret_path ? &found_path : NULL); + if (r < 0) + return r; + + if (ret_file) { + FILE *f = take_fdopen(&fd, mode); if (!f) return -errno; - if (ret_path) { - r = path_simplify_alloc(filename, ret_path); - if (r < 0) - return r; - } - - *ret = TAKE_PTR(f); - return 0; + *ret_file = f; } - s = strv_split_nulstr(search); - if (!s) + if (ret_path) + *ret_path = TAKE_PTR(found_path); + + return 0; +} + +int search_and_fopen( + const char *path, + const char *mode, + const char *root, + const char **search, + FILE **ret_file, + char **ret_path) { + + _cleanup_strv_free_ char **copy = NULL; + + assert(path); + assert(mode || !ret_file); + + copy = strv_copy((char**) search); + if (!copy) return -ENOMEM; - return search_and_fopen_internal(filename, mode, root, s, ret, ret_path); + return search_and_fopen_internal(path, mode, root, copy, ret_file, ret_path); +} + +int search_and_fopen_nulstr( + const char *path, + const char *mode, + const char *root, + const char *search, + FILE **ret_file, + char **ret_path) { + + _cleanup_strv_free_ char **l = NULL; + + assert(path); + assert(mode || !ret_file); + + l = strv_split_nulstr(search); + if (!l) + return -ENOMEM; + + return search_and_fopen_internal(path, mode, root, l, ret_file, ret_path); } int fflush_and_check(FILE *f) { diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 769bf394fd..cd18bcc117 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -129,8 +129,12 @@ static inline int fopen_unlocked(const char *path, const char *mode, FILE **ret) int fdopen_independent(int fd, const char *mode, FILE **ret); -int search_and_fopen(const char *path, const char *mode, const char *root, const char **search, FILE **ret, char **ret_path); -int search_and_fopen_nulstr(const char *path, const char *mode, const char *root, const char *search, FILE **ret, char **ret_path); +int search_and_open(const char *path, int mode, const char *root, char **search, int *ret_fd, char **ret_path); +static inline int search_and_access(const char *path, int mode, const char *root, char**search, char **ret_path) { + return search_and_open(path, mode, root, search, NULL, ret_path); +} +int search_and_fopen(const char *path, const char *mode, const char *root, const char **search, FILE **ret_file, char **ret_path); +int search_and_fopen_nulstr(const char *path, const char *mode, const char *root, const char *search, FILE **ret_file, char **ret_path); int fflush_and_check(FILE *f); int fflush_sync_and_check(FILE *f); diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 0eb7b07331..6efa8f2939 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -582,12 +582,23 @@ TEST(search_and_fopen) { f = safe_fclose(f); p = mfree(p); + r = search_and_fopen(basename(name), NULL, NULL, (const char**) dirs, NULL, &p); + assert_se(r >= 0); + assert_se(e = path_startswith(p, "/tmp/")); + assert_se(streq(basename(name), e)); + p = mfree(p); + r = search_and_fopen(name, "re", NULL, (const char**) dirs, &f, &p); assert_se(r >= 0); assert_se(path_equal(name, p)); f = safe_fclose(f); p = mfree(p); + r = search_and_fopen(name, NULL, NULL, (const char**) dirs, NULL, &p); + assert_se(r >= 0); + assert_se(path_equal(name, p)); + p = mfree(p); + r = search_and_fopen(basename(name), "re", "/", (const char**) dirs, &f, &p); assert_se(r >= 0); assert_se(e = path_startswith(p, "/tmp/")); @@ -595,16 +606,28 @@ TEST(search_and_fopen) { f = safe_fclose(f); p = mfree(p); + r = search_and_fopen(basename(name), NULL, "/", (const char**) dirs, NULL, &p); + assert_se(r >= 0); + assert_se(e = path_startswith(p, "/tmp/")); + assert_se(streq(basename(name), e)); + p = mfree(p); + r = search_and_fopen("/a/file/which/does/not/exist/i/guess", "re", NULL, (const char**) dirs, &f, &p); assert_se(r == -ENOENT); + r = search_and_fopen("/a/file/which/does/not/exist/i/guess", NULL, NULL, (const char**) dirs, NULL, &p); + assert_se(r == -ENOENT); r = search_and_fopen("afilewhichdoesnotexistiguess", "re", NULL, (const char**) dirs, &f, &p); assert_se(r == -ENOENT); + r = search_and_fopen("afilewhichdoesnotexistiguess", NULL, NULL, (const char**) dirs, NULL, &p); + assert_se(r == -ENOENT); r = unlink(name); assert_se(r == 0); r = search_and_fopen(basename(name), "re", NULL, (const char**) dirs, &f, &p); assert_se(r == -ENOENT); + r = search_and_fopen(basename(name), NULL, NULL, (const char**) dirs, NULL, &p); + assert_se(r == -ENOENT); } TEST(search_and_fopen_nulstr) { From 19c58989f7f0f84577f0fc63cdccdb01fa97673f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Sep 2023 18:08:48 +0200 Subject: [PATCH 02/15] repart: don't wipe the disk image if we just created it fresh Let's avoid some unnecessary work if we the image is freshly created anyway. --- src/partition/repart.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 1dd05b3bde..9f9acdff1e 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -3356,6 +3356,9 @@ static int context_wipe_and_discard(Context *context) { assert(context); + if (arg_empty == EMPTY_CREATE) /* If we just created the image, no need to wipe */ + return 0; + /* Wipe and discard the contents of all partitions we are about to create. We skip the discarding if * we were supposed to start from scratch anyway, as in that case we just discard the whole block * device in one go early on. */ @@ -5420,7 +5423,8 @@ static int context_write_partition_table(Context *context) { log_info("Applying changes."); - if (context->from_scratch) { + if (context->from_scratch && arg_empty != EMPTY_CREATE) { + /* Erase everything if we operate from scratch, except if the image was just created anyway, and thus is definitely empty. */ r = context_wipe_range(context, 0, context->total); if (r < 0) return r; From 6c05395e82448bdf92079e1d04507ea9af6ed41b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Sep 2023 18:20:41 +0200 Subject: [PATCH 03/15] repart: disable pager in --dry-run=no mode When --dry-run=yes is used it makes sense to spawn a pager to look at the report it provides you with about what it is about to do. Hoewver, when we are actually doing it, then the output is more in the category of "logs" than "review material", and logs we generally don't page when we generate them. --- src/partition/repart.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/partition/repart.c b/src/partition/repart.c index 9f9acdff1e..9b56444404 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -6806,6 +6806,10 @@ static int parse_argv(int argc, char *argv[]) { else if (dry_run >= 0) arg_dry_run = dry_run; + /* Disable pager once we are not just reviewing, but doing things. */ + if (!arg_dry_run) + arg_pager_flags |= PAGER_DISABLE; + if (arg_empty == EMPTY_CREATE && (arg_size == UINT64_MAX && !arg_size_auto)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "If --empty=create is specified, --size= must be specified, too."); From 243dd1e9fac1277f4877a42a50b6dcedd0c8a725 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Sep 2023 16:09:50 +0200 Subject: [PATCH 04/15] repart: rework --empty= handling a bit Introduce a new enum value EMPTY_UNSET to which arg_empty now is set initially. Only after we finished parsing the command line we'll now set this to EMPTY_REFUSE as before. This prepares ground for later changes, where we then can make different decisions after havig all input from the command line. As of now this doesn't change behaviour of systemd-repart, it just rearranges things a bit. --- src/partition/repart.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 9b56444404..8ba6a9cb05 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -108,12 +108,13 @@ * waste 3K per partition, which is probably fine. */ static enum { + EMPTY_UNSET, /* no choice has been made yet */ EMPTY_REFUSE, /* refuse empty disks, never create a partition table */ EMPTY_ALLOW, /* allow empty disks, create partition table if necessary */ EMPTY_REQUIRE, /* require an empty disk, create a partition table */ EMPTY_FORCE, /* make disk empty, erase everything, create a partition table always */ EMPTY_CREATE, /* create disk as loopback file, create a partition table always */ -} arg_empty = EMPTY_REFUSE; +} arg_empty = EMPTY_UNSET; typedef enum FilterPartitionType { FILTER_PARTITIONS_NONE, @@ -2416,6 +2417,9 @@ static int context_load_partition_table(Context *context) { /* Always reinitiaize the disk, don't consider what there was on the disk before */ from_scratch = true; break; + + case EMPTY_UNSET: + assert_not_reached(); } if (from_scratch) { @@ -6481,7 +6485,9 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_EMPTY: - if (isempty(optarg) || streq(optarg, "refuse")) + if (isempty(optarg)) + arg_empty = EMPTY_UNSET; + else if (streq(optarg, "refuse")) arg_empty = EMPTY_REFUSE; else if (streq(optarg, "allow")) arg_empty = EMPTY_ALLOW; @@ -6795,6 +6801,9 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected at most one argument, the path to the block device."); + if (arg_empty == EMPTY_UNSET) /* default to refuse mode, if not otherwise specified */ + arg_empty = EMPTY_REFUSE; + if (arg_factory_reset > 0 && IN_SET(arg_empty, EMPTY_FORCE, EMPTY_REQUIRE, EMPTY_CREATE)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Combination of --factory-reset=yes and --empty=force/--empty=require/--empty=create is invalid."); From ba6bd342fae4fed775f7dc053ad5b88391a60d4c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Sep 2023 16:16:03 +0200 Subject: [PATCH 05/15] repart: get rid of redundant variable 'dry_run' There's only one case where we set 'dry_run', which is when --empty=create is set. Hence simplify things and directly check for that. --- src/partition/repart.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 8ba6a9cb05..0948514455 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -6455,7 +6455,7 @@ static int parse_argv(int argc, char *argv[]) { {} }; - int c, r, dry_run = -1; + int c, r; assert(argc >= 0); assert(argv); @@ -6495,14 +6495,9 @@ static int parse_argv(int argc, char *argv[]) { arg_empty = EMPTY_REQUIRE; else if (streq(optarg, "force")) arg_empty = EMPTY_FORCE; - else if (streq(optarg, "create")) { + else if (streq(optarg, "create")) arg_empty = EMPTY_CREATE; - - if (dry_run < 0) - dry_run = false; /* Imply --dry-run=no if we create the loopback file - * anew. After all we cannot really break anyone's - * partition tables that way. */ - } else + else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse --empty= parameter: %s", optarg); break; @@ -6812,8 +6807,9 @@ static int parse_argv(int argc, char *argv[]) { arg_dry_run = true; /* When --can-factory-reset is specified we don't make changes, hence * non-dry-run mode makes no sense. Thus, imply dry run mode so that we * open things strictly read-only. */ - else if (dry_run >= 0) - arg_dry_run = dry_run; + else if (arg_empty == EMPTY_CREATE) + arg_dry_run = false; /* Imply --dry-run=no if we create the loopback file anew. After all we + * cannot really break anyone's partition tables that way. */ /* Disable pager once we are not just reviewing, but doing things. */ if (!arg_dry_run) From 248f0186c12ff13095fae5ffb989f4bf49929b23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Sep 2023 16:18:22 +0200 Subject: [PATCH 06/15] repart: drop redundant () --- src/partition/repart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 0948514455..1fdc628f95 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -6815,7 +6815,7 @@ static int parse_argv(int argc, char *argv[]) { if (!arg_dry_run) arg_pager_flags |= PAGER_DISABLE; - if (arg_empty == EMPTY_CREATE && (arg_size == UINT64_MAX && !arg_size_auto)) + if (arg_empty == EMPTY_CREATE && arg_size == UINT64_MAX && !arg_size_auto) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "If --empty=create is specified, --size= must be specified, too."); From 607343a1acd563138b7561fff0da24c5bb654909 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Sep 2023 16:24:48 +0200 Subject: [PATCH 07/15] repart: add new --copy-source= switch This specifies a directory to which CopyFiles= is considered relative. If unset defaults to the --root=/--image= setting, or host / otherwise. This is very similar to --root= but is much more focussed: it is really and exclusively about CopyFiles= (and related settings such as ExcludeFiles=) and does not affect any of the settings, i.e. it doesn't affect CopyBlocks=, the machine ID/seed handling, or where definitions are read from. In fact, --root= and --copy-source= may be combined for example to use the machine ID and similar from one tree, but the copy the files from another. --- man/repart.d.xml | 9 +++++---- man/systemd-repart.xml | 16 ++++++++++++++++ src/partition/repart.c | 40 +++++++++++++++++++++++++++++----------- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/man/repart.d.xml b/man/repart.d.xml index cfdd39d5eb..79908a0805 100644 --- a/man/repart.d.xml +++ b/man/repart.d.xml @@ -468,10 +468,11 @@ This option cannot be combined with CopyBlocks=. When - systemd-repart8 - is invoked with the or command line switches the - source paths specified are taken relative to the specified root directory or disk image root. - + systemd-repart8 is + invoked with the command line switch the file paths are taken + relative to the specified directory. If is not used, but the + or switches are used, the source paths are taken + relative to the specified root directory or disk image root. diff --git a/man/systemd-repart.xml b/man/systemd-repart.xml index 89d7f2c285..fb7b5496af 100644 --- a/man/systemd-repart.xml +++ b/man/systemd-repart.xml @@ -276,6 +276,9 @@ so that the tool operates on the configuration and machine ID stored in the root file system later transitioned into itself. + See for a more restricted option that only affects + CopyFiles=. + @@ -500,6 +503,19 @@ + + PATH + PATH + + Specifies a source directory all CopyFiles= source paths shall be + considered relative to. This is similar to , but exclusively applies to the + CopyFiles= setting. If and + are used in combination the former applies as usual, except for + CopyFiles= where the latter takes precedence. + + + + diff --git a/src/partition/repart.c b/src/partition/repart.c index 1fdc628f95..770f7c48ff 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -162,6 +162,7 @@ static ImagePolicy *arg_image_policy = NULL; static Architecture arg_architecture = _ARCHITECTURE_INVALID; static int arg_offline = -1; static char **arg_copy_from = NULL; +static char *arg_copy_source = NULL; STATIC_DESTRUCTOR_REGISTER(arg_root, freep); STATIC_DESTRUCTOR_REGISTER(arg_image, freep); @@ -175,6 +176,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_tpm2_public_key, freep); STATIC_DESTRUCTOR_REGISTER(arg_filter_partitions, freep); STATIC_DESTRUCTOR_REGISTER(arg_image_policy, image_policy_freep); STATIC_DESTRUCTOR_REGISTER(arg_copy_from, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_copy_source, freep); typedef struct FreeArea FreeArea; @@ -4274,11 +4276,11 @@ static int add_exclude_path(const char *path, Hashmap **denylist, DenyType type) if (!st) return log_oom(); - r = chase_and_stat(path, arg_root, CHASE_PREFIX_ROOT, NULL, st); + r = chase_and_stat(path, arg_copy_source, CHASE_PREFIX_ROOT, NULL, st); if (r == -ENOENT) return 0; if (r < 0) - return log_error_errno(r, "Failed to stat source file '%s/%s': %m", strempty(arg_root), path); + return log_error_errno(r, "Failed to stat source file '%s/%s': %m", strempty(arg_copy_source), path); r = hashmap_ensure_put(denylist, &inode_hash_ops, st, INT_TO_PTR(type)); if (r == -EEXIST) @@ -4402,11 +4404,11 @@ static int add_subvolume_path(const char *path, Set **subvolumes) { if (!st) return log_oom(); - r = chase_and_stat(path, arg_root, CHASE_PREFIX_ROOT, NULL, st); + r = chase_and_stat(path, arg_copy_source, CHASE_PREFIX_ROOT, NULL, st); if (r == -ENOENT) return 0; if (r < 0) - return log_error_errno(r, "Failed to stat source file '%s/%s': %m", strempty(arg_root), path); + return log_error_errno(r, "Failed to stat source file '%s/%s': %m", strempty(arg_copy_source), path); r = set_ensure_consume(subvolumes, &inode_hash_ops, TAKE_PTR(st)); if (r < 0) @@ -4469,9 +4471,9 @@ static int do_copy_files(Context *context, Partition *p, const char *root) { if (rfd < 0) return rfd; - sfd = chase_and_open(*source, arg_root, CHASE_PREFIX_ROOT, O_PATH|O_DIRECTORY|O_CLOEXEC|O_NOCTTY, NULL); + sfd = chase_and_open(*source, arg_copy_source, CHASE_PREFIX_ROOT, O_PATH|O_DIRECTORY|O_CLOEXEC|O_NOCTTY, NULL); if (sfd < 0) - return log_error_errno(sfd, "Failed to open source file '%s%s': %m", strempty(arg_root), *source); + return log_error_errno(sfd, "Failed to open source file '%s%s': %m", strempty(arg_copy_source), *source); (void) copy_xattr(sfd, NULL, rfd, NULL, COPY_ALL_XATTRS); (void) copy_access(sfd, rfd); @@ -4493,9 +4495,9 @@ static int do_copy_files(Context *context, Partition *p, const char *root) { if (r < 0) return r; - sfd = chase_and_open(*source, arg_root, CHASE_PREFIX_ROOT, O_CLOEXEC|O_NOCTTY, NULL); + sfd = chase_and_open(*source, arg_copy_source, CHASE_PREFIX_ROOT, O_CLOEXEC|O_NOCTTY, NULL); if (sfd < 0) - return log_error_errno(sfd, "Failed to open source file '%s%s': %m", strempty(arg_root), *source); + return log_error_errno(sfd, "Failed to open source file '%s%s': %m", strempty(arg_copy_source), *source); r = fd_verify_regular(sfd); if (r < 0) { @@ -4541,7 +4543,7 @@ static int do_copy_files(Context *context, Partition *p, const char *root) { denylist, subvolumes_by_source_inode); if (r < 0) return log_error_errno(r, "Failed to copy '%s%s' to '%s%s': %m", - strempty(arg_root), *source, strempty(root), *target); + strempty(arg_copy_source), *source, strempty(root), *target); } else { _cleanup_free_ char *dn = NULL, *fn = NULL; @@ -4572,7 +4574,7 @@ static int do_copy_files(Context *context, Partition *p, const char *root) { r = copy_bytes(sfd, tfd, UINT64_MAX, COPY_REFLINK|COPY_HOLES|COPY_SIGINT|COPY_TRUNCATE); if (r < 0) - return log_error_errno(r, "Failed to copy '%s' to '%s%s': %m", *source, strempty(arg_root), *target); + return log_error_errno(r, "Failed to copy '%s' to '%s%s': %m", *source, strempty(arg_copy_source), *target); (void) copy_xattr(sfd, NULL, tfd, NULL, COPY_ALL_XATTRS); (void) copy_access(sfd, tfd); @@ -6372,6 +6374,7 @@ static int help(void) { " --sector-size=SIZE Set the logical sector size for the image\n" " --architecture=ARCH Set the generic architecture for the image\n" " --offline=BOOL Whether to build the image offline\n" + " -s --copy-source=PATH Specify the primary source tree to copy files from\n" " --copy-from=IMAGE Copy partitions from the given image(s)\n" "\nSee the %s for details.\n", program_invocation_short_name, @@ -6417,6 +6420,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_ARCHITECTURE, ARG_OFFLINE, ARG_COPY_FROM, + ARG_COPY_SOURCE, }; static const struct option options[] = { @@ -6452,6 +6456,7 @@ static int parse_argv(int argc, char *argv[]) { { "architecture", required_argument, NULL, ARG_ARCHITECTURE }, { "offline", required_argument, NULL, ARG_OFFLINE }, { "copy-from", required_argument, NULL, ARG_COPY_FROM }, + { "copy-source", required_argument, NULL, 's' }, {} }; @@ -6460,7 +6465,7 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, "hs:", options, NULL)) >= 0) switch (c) { @@ -6785,6 +6790,12 @@ static int parse_argv(int argc, char *argv[]) { break; } + case 's': + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_copy_source); + if (r < 0) + return r; + break; + case '?': return -EINVAL; @@ -7345,6 +7356,13 @@ static int run(int argc, char *argv[]) { } } + if (!arg_copy_source && arg_root) { + /* If no explicit copy source is specified, then use --root=/--image= */ + arg_copy_source = strdup(arg_root); + if (!arg_copy_source) + return log_oom(); + } + context = context_new(arg_seed); if (!context) return log_oom(); From a121b331e37682341152b51269cf8ce00e68d755 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Sep 2023 18:16:59 +0200 Subject: [PATCH 08/15] repart: make it easy to generate DDIs This adds --make-ddi=confext, --make-ddi=sysext, --make-ddi=portable, to make it really easiy to generate DDIs of the specified class. It it's ultimately just a fancy wrapper around some defaults and in particular --definitions=. This makes it very easy to generate a confext: $ systemd-repart -C --private-key=privkey.pem --certificate=cert.crt -s mytree/ mytree.confext.raw --- man/systemd-repart.xml | 71 ++++++++++++++- meson.build | 1 + .../definitions/confext.repart.d/10-root.conf | 15 ++++ .../confext.repart.d/20-root-verity.conf | 13 +++ .../confext.repart.d/30-root-verity-sig.conf | 13 +++ .../portable.repart.d/10-root.conf | 15 ++++ .../portable.repart.d/20-root-verity.conf | 13 +++ .../portable.repart.d/30-root-verity-sig.conf | 13 +++ .../definitions/sysext.repart.d/10-root.conf | 15 ++++ .../sysext.repart.d/20-root-verity.conf | 13 +++ .../sysext.repart.d/30-root-verity-sig.conf | 13 +++ src/partition/meson.build | 12 +++ src/partition/repart.c | 86 ++++++++++++++++++- 13 files changed, 289 insertions(+), 4 deletions(-) create mode 100644 src/partition/definitions/confext.repart.d/10-root.conf create mode 100644 src/partition/definitions/confext.repart.d/20-root-verity.conf create mode 100644 src/partition/definitions/confext.repart.d/30-root-verity-sig.conf create mode 100644 src/partition/definitions/portable.repart.d/10-root.conf create mode 100644 src/partition/definitions/portable.repart.d/20-root-verity.conf create mode 100644 src/partition/definitions/portable.repart.d/30-root-verity-sig.conf create mode 100644 src/partition/definitions/sysext.repart.d/10-root.conf create mode 100644 src/partition/definitions/sysext.repart.d/20-root-verity.conf create mode 100644 src/partition/definitions/sysext.repart.d/30-root-verity-sig.conf diff --git a/man/systemd-repart.xml b/man/systemd-repart.xml index fb7b5496af..d05b74f57d 100644 --- a/man/systemd-repart.xml +++ b/man/systemd-repart.xml @@ -516,6 +516,47 @@ + + TYPE + + Takes one of sysext, confext or + portable. Generates a Discoverable Disk Image (DDI) for a system extension + (sysext, see + systemd-sysext8 + for details), configuration extension (confext) or portable service. The generated image will consist + of a signed Verity erofs file system as root partition. In this mode of operation + the partition definitions in /usr/lib/repart.d/*.conf and related directories + are not read, and is not supported, as appropriate definitions for + the selected DDI class will be chosen automatically. + + Must be used in conjunction with to specify the file hierarchy + to populate the DDI with. The specified directory should contain an etc/ + subdirectory if confext is selected. If sysext is selected it + should contain either a usr/ or opt/ directory, or both. If + portable is used a full OS file hierarchy can be provided. + + This option implies , and + (the latter two can be overriden). + + The private key and certificate for signing the DDI must be specified via the + and switches. + + + + + + + + + + Shortcuts for , + , , + respectively. + + + + @@ -530,13 +571,41 @@ On success, 0 is returned, a non-zero failure code otherwise. + + Example + + + Generate a configuration extension image + + The following creates a configuration extension DDI (confext) for an + /etc/motd update. + + mkdir tree tree/etc tree/etc/extension-release.d +echo "Hello World" > tree/etc/motd +cat > tree/etc/extension-release.d/extension-release.my-motd <<EOF +ID=fedora +VERSION_ID=38 +IMAGE_ID=my-motd +IMAGE_VERSION=7 +EOF +systemd-repart -C --private-key=privkey.pem --certificate=cert.crt -s tree/ /var/lib/confexts/my-motd.confext.raw +systemd-confext refresh + + The DDI generated that way may be applied to the system with + systemd-confext1. + + + + See Also systemd1, repart.d5, machine-id5, - systemd-cryptenroll1 + systemd-cryptenroll1, + portablectl1, + systemd-sysext8 diff --git a/meson.build b/meson.build index 1e59d353ee..88bc1367e0 100644 --- a/meson.build +++ b/meson.build @@ -171,6 +171,7 @@ systemdstatedir = localstatedir / 'lib/systemd' catalogstatedir = systemdstatedir / 'catalog' randomseeddir = localstatedir / 'lib/systemd' profiledir = libexecdir / 'portable' / 'profile' +repartdefinitionsdir = libexecdir / 'repart/definitions' ntpservicelistdir = prefixdir / 'lib/systemd/ntp-units.d' credstoredir = prefixdir / 'lib/credstore' diff --git a/src/partition/definitions/confext.repart.d/10-root.conf b/src/partition/definitions/confext.repart.d/10-root.conf new file mode 100644 index 0000000000..e41dc0578b --- /dev/null +++ b/src/partition/definitions/confext.repart.d/10-root.conf @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root +Format=erofs +CopyFiles=/etc/ +Verity=data +VerityMatchKey=root diff --git a/src/partition/definitions/confext.repart.d/20-root-verity.conf b/src/partition/definitions/confext.repart.d/20-root-verity.conf new file mode 100644 index 0000000000..437d88e068 --- /dev/null +++ b/src/partition/definitions/confext.repart.d/20-root-verity.conf @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root-verity +Verity=hash +VerityMatchKey=root diff --git a/src/partition/definitions/confext.repart.d/30-root-verity-sig.conf b/src/partition/definitions/confext.repart.d/30-root-verity-sig.conf new file mode 100644 index 0000000000..df160154a7 --- /dev/null +++ b/src/partition/definitions/confext.repart.d/30-root-verity-sig.conf @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root-verity-sig +Verity=signature +VerityMatchKey=root diff --git a/src/partition/definitions/portable.repart.d/10-root.conf b/src/partition/definitions/portable.repart.d/10-root.conf new file mode 100644 index 0000000000..78758002f5 --- /dev/null +++ b/src/partition/definitions/portable.repart.d/10-root.conf @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root +Format=erofs +CopyFiles=/ +Verity=data +VerityMatchKey=root diff --git a/src/partition/definitions/portable.repart.d/20-root-verity.conf b/src/partition/definitions/portable.repart.d/20-root-verity.conf new file mode 100644 index 0000000000..437d88e068 --- /dev/null +++ b/src/partition/definitions/portable.repart.d/20-root-verity.conf @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root-verity +Verity=hash +VerityMatchKey=root diff --git a/src/partition/definitions/portable.repart.d/30-root-verity-sig.conf b/src/partition/definitions/portable.repart.d/30-root-verity-sig.conf new file mode 100644 index 0000000000..df160154a7 --- /dev/null +++ b/src/partition/definitions/portable.repart.d/30-root-verity-sig.conf @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root-verity-sig +Verity=signature +VerityMatchKey=root diff --git a/src/partition/definitions/sysext.repart.d/10-root.conf b/src/partition/definitions/sysext.repart.d/10-root.conf new file mode 100644 index 0000000000..41a7ca56d7 --- /dev/null +++ b/src/partition/definitions/sysext.repart.d/10-root.conf @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root +Format=erofs +CopyFiles=/usr/ /opt/ +Verity=data +VerityMatchKey=root diff --git a/src/partition/definitions/sysext.repart.d/20-root-verity.conf b/src/partition/definitions/sysext.repart.d/20-root-verity.conf new file mode 100644 index 0000000000..437d88e068 --- /dev/null +++ b/src/partition/definitions/sysext.repart.d/20-root-verity.conf @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root-verity +Verity=hash +VerityMatchKey=root diff --git a/src/partition/definitions/sysext.repart.d/30-root-verity-sig.conf b/src/partition/definitions/sysext.repart.d/30-root-verity-sig.conf new file mode 100644 index 0000000000..df160154a7 --- /dev/null +++ b/src/partition/definitions/sysext.repart.d/30-root-verity-sig.conf @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Partition] +Type=root-verity-sig +Verity=signature +VerityMatchKey=root diff --git a/src/partition/meson.build b/src/partition/meson.build index 19f49f606e..78cde2ff52 100644 --- a/src/partition/meson.build +++ b/src/partition/meson.build @@ -48,3 +48,15 @@ executables += [ 'install' : have_standalone_binaries, }, ] + +if conf.get('ENABLE_REPART') == 1 + install_data('definitions/confext.repart.d/10-root.conf', install_dir : repartdefinitionsdir / 'confext.repart.d') + install_data('definitions/confext.repart.d/20-root-verity.conf', install_dir : repartdefinitionsdir / 'confext.repart.d') + install_data('definitions/confext.repart.d/30-root-verity-sig.conf', install_dir : repartdefinitionsdir / 'confext.repart.d') + install_data('definitions/portable.repart.d/10-root.conf', install_dir : repartdefinitionsdir / 'portable.repart.d') + install_data('definitions/portable.repart.d/20-root-verity.conf', install_dir : repartdefinitionsdir / 'portable.repart.d') + install_data('definitions/portable.repart.d/30-root-verity-sig.conf', install_dir : repartdefinitionsdir / 'portable.repart.d') + install_data('definitions/sysext.repart.d/10-root.conf', install_dir : repartdefinitionsdir / 'sysext.repart.d') + install_data('definitions/sysext.repart.d/20-root-verity.conf', install_dir : repartdefinitionsdir / 'sysext.repart.d') + install_data('definitions/sysext.repart.d/30-root-verity-sig.conf', install_dir : repartdefinitionsdir / 'sysext.repart.d') +endif diff --git a/src/partition/repart.c b/src/partition/repart.c index 770f7c48ff..855e908d75 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -163,6 +163,7 @@ static Architecture arg_architecture = _ARCHITECTURE_INVALID; static int arg_offline = -1; static char **arg_copy_from = NULL; static char *arg_copy_source = NULL; +static char *arg_make_ddi = NULL; STATIC_DESTRUCTOR_REGISTER(arg_root, freep); STATIC_DESTRUCTOR_REGISTER(arg_image, freep); @@ -177,6 +178,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_filter_partitions, freep); STATIC_DESTRUCTOR_REGISTER(arg_image_policy, image_policy_freep); STATIC_DESTRUCTOR_REGISTER(arg_copy_from, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_copy_source, freep); +STATIC_DESTRUCTOR_REGISTER(arg_make_ddi, freep); typedef struct FreeArea FreeArea; @@ -6376,6 +6378,9 @@ static int help(void) { " --offline=BOOL Whether to build the image offline\n" " -s --copy-source=PATH Specify the primary source tree to copy files from\n" " --copy-from=IMAGE Copy partitions from the given image(s)\n" + " -S --make-ddi=sysext Make a system extension DDI\n" + " -C --make-ddi=confext Make a configuration extension DDI\n" + " -P --make-ddi=portable Make a portable service DDI\n" "\nSee the %s for details.\n", program_invocation_short_name, ansi_highlight(), @@ -6420,7 +6425,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_ARCHITECTURE, ARG_OFFLINE, ARG_COPY_FROM, - ARG_COPY_SOURCE, + ARG_MAKE_DDI, }; static const struct option options[] = { @@ -6457,6 +6462,7 @@ static int parse_argv(int argc, char *argv[]) { { "offline", required_argument, NULL, ARG_OFFLINE }, { "copy-from", required_argument, NULL, ARG_COPY_FROM }, { "copy-source", required_argument, NULL, 's' }, + { "make-ddi", required_argument, NULL, ARG_MAKE_DDI }, {} }; @@ -6465,7 +6471,7 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "hs:", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, "hs:SCP", options, NULL)) >= 0) switch (c) { @@ -6796,6 +6802,33 @@ static int parse_argv(int argc, char *argv[]) { return r; break; + case ARG_MAKE_DDI: + if (!filename_is_valid(optarg)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid DDI type: %s", optarg); + + r = free_and_strdup_warn(&arg_make_ddi, optarg); + if (r < 0) + return r; + break; + + case 'S': + r = free_and_strdup_warn(&arg_make_ddi, "sysext"); + if (r < 0) + return r; + break; + + case 'C': + r = free_and_strdup_warn(&arg_make_ddi, "confext"); + if (r < 0) + return r; + break; + + case 'P': + r = free_and_strdup_warn(&arg_make_ddi, "portable"); + if (r < 0) + return r; + break; + case '?': return -EINVAL; @@ -6807,6 +6840,38 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected at most one argument, the path to the block device."); + if (arg_make_ddi) { + if (arg_definitions) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Combination of --make-ddi= and --definitions= is not supported."); + if (!IN_SET(arg_empty, EMPTY_UNSET, EMPTY_CREATE)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Combination of --make-ddi= and --empty=%s is not supported.", + arg_empty == EMPTY_REFUSE ? "refuse" : + arg_empty == EMPTY_ALLOW ? "allow" : + arg_empty == EMPTY_REQUIRE ? "require" : "force"); + + /* Imply automatic sizing in DDI mode */ + if (arg_size == UINT64_MAX) + arg_size_auto = true; + + if (!arg_copy_source) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No --copy-source= specified, refusing."); + + r = dir_is_empty(arg_copy_source, /* ignore_hidden_or_backup= */ false); + if (r < 0) + return log_error_errno(r, "Failed to determine if '%s' is empty: %m", arg_copy_source); + if (r > 0) + return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "Source directory '%s' is empty, refusing to create empty image.", arg_copy_source); + + if (sd_id128_is_null(arg_seed) && !arg_randomize) { + /* We don't want that /etc/machine-id leaks into any image built this way, hence + * let's randomize the seed if not specified explicitly */ + log_notice("No seed value specified, randomizing generated UUIDs, resulting image will not be reproducible."); + arg_randomize = true; + } + + arg_empty = EMPTY_CREATE; + } + if (arg_empty == EMPTY_UNSET) /* default to refuse mode, if not otherwise specified */ arg_empty = EMPTY_REFUSE; @@ -7371,7 +7436,22 @@ static int run(int argc, char *argv[]) { if (r < 0) return r; - strv_uniq(arg_definitions); + if (arg_make_ddi) { + _cleanup_free_ char *d = NULL, *dp = NULL; + assert(!arg_definitions); + + d = strjoin(arg_make_ddi, ".repart.d/"); + if (!d) + return log_oom(); + + r = search_and_access(d, F_OK, arg_root, CONF_PATHS_USR_STRV("systemd/repart/definitions"), &dp); + if (r < 0) + return log_error_errno(errno, "DDI type '%s' is not defined: %m", arg_make_ddi); + + if (strv_consume(&arg_definitions, TAKE_PTR(dp)) < 0) + return log_oom(); + } else + strv_uniq(arg_definitions); r = context_read_definitions(context); if (r < 0) From a747994b451cf8dea393c9293f802777bc314bb7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Sep 2023 18:22:24 +0200 Subject: [PATCH 09/15] discover-image: nicely support .sysext.raw + .confext.raw suffix for DDIs Back at Plumbers 2022 we discussed that we should push people towards using the .sysext.raw and .confext.raw for their DDIs if they are extension images. Let's actually support that, by chopping off these suffixes when turning file names into "pretty image names". --- src/shared/discover-image.c | 69 ++++++++++++++++++++----------------- src/shared/import-util.c | 2 ++ 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index f94fac1b41..50662037de 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -85,6 +85,13 @@ static const char* const image_search_path_initrd[_IMAGE_CLASS_MAX] = { "/.extra/sysext\0" /* put sysext picked up by systemd-stub last, since not trusted */ }; +static const char* image_class_suffix_table[_IMAGE_CLASS_MAX] = { + [IMAGE_SYSEXT] = ".sysext", + [IMAGE_CONFEXT] = ".confext", +}; + +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(image_class_suffix, ImageClass); + static Image *image_free(Image *i) { assert(i); @@ -201,29 +208,36 @@ static int image_new( return 0; } -static int extract_pretty(const char *path, const char *suffix, char **ret) { +static int extract_pretty( + const char *path, + const char *class_suffix, + const char *format_suffix, + char **ret) { + _cleanup_free_ char *name = NULL; - const char *p; + int r; assert(path); assert(ret); - p = last_path_component(path); + r = path_extract_filename(path, &name); + if (r < 0) + return r; - name = strdupcspn(p, "/"); - if (!name) - return -ENOMEM; - - if (suffix) { - char *e; - - e = endswith(name, suffix); - if (!e) + if (format_suffix) { + char *e = endswith(name, format_suffix); + if (!e) /* Format suffix is required */ return -EINVAL; *e = 0; } + if (class_suffix) { + char *e = endswith(name, class_suffix); + if (e) /* Class suffix is optional */ + *e = 0; + } + if (!image_name_is_valid(name)) return -EINVAL; @@ -282,7 +296,7 @@ static int image_make( return 0; if (!pretty) { - r = extract_pretty(filename, NULL, &pretty_buffer); + r = extract_pretty(filename, image_class_suffix_to_string(c), NULL, &pretty_buffer); if (r < 0) return r; @@ -369,7 +383,7 @@ static int image_make( (void) fd_getcrtime_at(dfd, filename, AT_SYMLINK_FOLLOW, &crtime); if (!pretty) { - r = extract_pretty(filename, ".raw", &pretty_buffer); + r = extract_pretty(filename, image_class_suffix_to_string(c), ".raw", &pretty_buffer); if (r < 0) return r; @@ -403,7 +417,7 @@ static int image_make( return 0; if (!pretty) { - r = extract_pretty(filename, NULL, &pretty_buffer); + r = extract_pretty(filename, NULL, NULL, &pretty_buffer); if (r < 0) return r; @@ -592,8 +606,7 @@ int image_discover( FOREACH_DIRENT_ALL(de, d, return -errno) { _cleanup_(image_unrefp) Image *image = NULL; - _cleanup_free_ char *truncated = NULL; - const char *pretty; + _cleanup_free_ char *pretty = NULL; struct stat st; int flags; @@ -610,22 +623,16 @@ int image_discover( return -errno; } - if (S_ISREG(st.st_mode)) { - const char *e; - - e = endswith(de->d_name, ".raw"); - if (!e) - continue; - - truncated = strndup(de->d_name, e - de->d_name); - if (!truncated) - return -ENOMEM; - - pretty = truncated; - } else if (S_ISDIR(st.st_mode) || S_ISBLK(st.st_mode)) - pretty = de->d_name; + if (S_ISREG(st.st_mode)) + r = extract_pretty(de->d_name, image_class_suffix_to_string(class), ".raw", &pretty); + else if (S_ISDIR(st.st_mode)) + r = extract_pretty(de->d_name, image_class_suffix_to_string(class), NULL, &pretty); + else if (S_ISBLK(st.st_mode)) + r = extract_pretty(de->d_name, NULL, NULL, &pretty); else continue; + if (r < 0) + return r; if (!image_name_is_valid(pretty)) continue; diff --git a/src/shared/import-util.c b/src/shared/import-util.c index ec89064506..9057b78d28 100644 --- a/src/shared/import-util.c +++ b/src/shared/import-util.c @@ -167,6 +167,8 @@ int raw_strip_suffixes(const char *p, char **ret) { ".xz\0" ".gz\0" ".bz2\0" + ".sysext.raw\0" + ".confext.raw\0" ".raw\0" ".qcow2\0" ".img\0" From 95515896f686106c5d10da25f7f72be662657019 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Sep 2023 18:24:11 +0200 Subject: [PATCH 10/15] sysext: fix some sysextisms in confext mode Nothing earth-shattering. Just make sure we never expose the string "sysext" in "confext" mode. --- src/sysext/sysext.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index 33bfb3dc80..784accc15d 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -591,13 +591,13 @@ static int merge_subprocess(Hashmap *images, const char *workspace) { /* Let's create the workspace if it's missing */ r = mkdir_p(workspace, 0700); if (r < 0) - return log_error_errno(r, "Failed to create /run/systemd/sysext: %m"); + return log_error_errno(r, "Failed to create '%s': %m", workspace); /* Let's mount a tmpfs to our workspace. This way we don't need to clean up the inodes we mount over, * but let the kernel do that entirely automatically, once our namespace dies. Note that this file * system won't be visible to anyone but us, since we opened our own namespace and then made the * /run/ hierarchy (which our workspace is contained in) MS_SLAVE, see above. */ - r = mount_nofollow_verbose(LOG_ERR, "sysext", workspace, "tmpfs", 0, "mode=0700"); + r = mount_nofollow_verbose(LOG_ERR, image_class_info[arg_image_class].short_identifier, workspace, "tmpfs", 0, "mode=0700"); if (r < 0) return r; From 37a38788015b51c5455bc97c30341e1f12285649 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Sep 2023 18:57:39 +0200 Subject: [PATCH 11/15] repart: say image file instead of loopback file After all we can now operate without loopback device, so it might be a big misleading. --- src/partition/repart.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 855e908d75..a38950c714 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -6838,7 +6838,7 @@ static int parse_argv(int argc, char *argv[]) { if (argc - optind > 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Expected at most one argument, the path to the block device."); + "Expected at most one argument, the path to the block device or image file."); if (arg_make_ddi) { if (arg_definitions) @@ -6918,11 +6918,11 @@ static int parse_argv(int argc, char *argv[]) { if (IN_SET(arg_empty, EMPTY_FORCE, EMPTY_REQUIRE, EMPTY_CREATE) && !arg_node && !arg_image) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "A path to a device node or loopback file must be specified when --empty=force, --empty=require or --empty=create are used."); + "A path to a device node or image file must be specified when --make-ddi=, --empty=force, --empty=require or --empty=create are used."); if (arg_split && !arg_node) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "A path to a loopback file must be specified when --split is used."); + "A path to an image file must be specified when --split is used."); if (arg_tpm2_public_key_pcr_mask_use_default && arg_tpm2_public_key) arg_tpm2_public_key_pcr_mask = INDEX_TO_MASK(uint32_t, TPM2_PCR_KERNEL_BOOT); From 489b0f515b196fe87fda8ade3cea58126cbf5cd5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Sep 2023 16:44:45 +0200 Subject: [PATCH 12/15] repart: relax rules on CopyFiles= a bit If a source dir doesn't exist, mention this, but continue. This is useful to permit generic definitions that apply to all kinds of trees, where some parts might remain unpopulated. --- src/partition/repart.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/partition/repart.c b/src/partition/repart.c index a38950c714..51706c839a 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -4498,6 +4498,10 @@ static int do_copy_files(Context *context, Partition *p, const char *root) { return r; sfd = chase_and_open(*source, arg_copy_source, CHASE_PREFIX_ROOT, O_CLOEXEC|O_NOCTTY, NULL); + if (sfd == -ENOENT) { + log_notice_errno(sfd, "Failed to open source file '%s%s', skipping: %m", strempty(arg_copy_source), *source); + continue; + } if (sfd < 0) return log_error_errno(sfd, "Failed to open source file '%s%s': %m", strempty(arg_copy_source), *source); From e59049d7a91ba1285b744cdfccf2856bab8c6f12 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Oct 2023 18:54:43 +0200 Subject: [PATCH 13/15] repart: add simple mechanism to override fstype choices This is very useful for quickly testing things when building DDIs, in particular in the CI, and trivial to add. --- docs/ENVIRONMENT.md | 3 +++ src/partition/repart.c | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 6396789e4a..65d28e6bfa 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -552,3 +552,6 @@ SYSTEMD_HOME_DEBUG_SUFFIX=foo \ * `$SYSTEMD_REPART_MKFS_OPTIONS_` – configure additional arguments to use for `mkfs` when formatting partition file systems. There's one variable for each of the supported file systems. + +* `$SYSTEMD_REPART_OVERRIDE_FSTYPE` – if set the value will override the file + system type specified in Format= lines in partition definition files. diff --git a/src/partition/repart.c b/src/partition/repart.c index 51706c839a..3d4baac295 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -1378,9 +1378,18 @@ static int config_parse_fstype( void *userdata) { char **fstype = ASSERT_PTR(data); + const char *e; assert(rvalue); + /* Let's provide an easy way to override the chosen fstype for file system partitions */ + e = secure_getenv("SYSTEMD_REPART_OVERRIDE_FSTYPE"); + if (e && !streq(rvalue, e)) { + log_syntax(unit, LOG_NOTICE, filename, line, 0, + "Overriding defined file system type '%s' with '%s'.", rvalue, e); + rvalue = e; + } + if (!filename_is_valid(rvalue)) return log_syntax(unit, LOG_ERR, filename, line, 0, "File system type is not valid, refusing: %s", rvalue); From 99d9edf0bde8741b693b8cab5fe117952ce4a1df Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Sep 2023 18:02:18 +0200 Subject: [PATCH 14/15] test: test the new --make-ddi= logic --- test/units/testsuite-50.sh | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh index 7bede7da1a..58be950d5f 100755 --- a/test/units/testsuite-50.sh +++ b/test/units/testsuite-50.sh @@ -2,6 +2,7 @@ # SPDX-License-Identifier: LGPL-2.1-or-later # -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- # ex: ts=8 sw=4 sts=4 et filetype=sh +# shellcheck disable=SC2233,SC2235 set -eux set -o pipefail @@ -624,4 +625,36 @@ systemd-sysext merge systemd-sysext unmerge systemctl status foo.service 2>&1 | grep -v -q -F "Warning" +# Test systemd-repart --make-ddi=: +if command -v mksquashfs >/dev/null 2>&1; then + + openssl req -config /dev/null -subj="/CN=waldo" -x509 -sha256 -nodes -days 365 -newkey rsa:4096 -keyout /tmp/test-50-privkey.key -out /tmp/test-50-cert.crt + + mkdir -p /tmp/test-50-confext/etc/extension-release.d/ + + echo "foobar50" > /tmp/test-50-confext/etc/waldo + + ( grep -e '^\(ID\|VERSION_ID\)=' /etc/os-release ; echo IMAGE_ID=waldo ; echo IMAGE_VERSION=7 ) > /tmp/test-50-confext/etc/extension-release.d/extension-release.waldo + + mkdir -p /run/confexts + + SYSTEMD_REPART_OVERRIDE_FSTYPE=squashfs systemd-repart -C -s /tmp/test-50-confext --certificate=/tmp/test-50-cert.crt --private-key=/tmp/test-50-privkey.key /run/confexts/waldo.confext.raw + rm -rf /tmp/test-50-confext + + mkdir -p /run/verity.d + cp /tmp/test-50-cert.crt /run/verity.d/ + systemd-dissect --mtree /run/confexts/waldo.confext.raw + + systemd-confext refresh + + read -r X < /etc/waldo + test "$X" = foobar50 + + rm /run/verity.d/test-50-cert.crt /run/confexts/waldo.confext.raw /tmp/test-50-cert.crt /tmp/test-50-privkey.key + + systemd-confext refresh + + (! test -f /tmp/test-50-confext/etc/waldo ) +fi + touch /testok From 634b847149d2385be475f50edc95bd565de1b68d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Oct 2023 17:10:31 +0200 Subject: [PATCH 15/15] repart: make arg_empty a proper enum with a string table Now that we translate in both directions (from enum to string, and string to enum) let's just make the thing a proper enum with a proper string table, like we generally do. --- src/partition/repart.c | 46 +++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 3d4baac295..c2eec29ca2 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -107,14 +107,16 @@ * sector size devices were generally assumed to have an even number of sectors, hence at the worst we'll * waste 3K per partition, which is probably fine. */ -static enum { +typedef enum EmptyMode { EMPTY_UNSET, /* no choice has been made yet */ EMPTY_REFUSE, /* refuse empty disks, never create a partition table */ EMPTY_ALLOW, /* allow empty disks, create partition table if necessary */ EMPTY_REQUIRE, /* require an empty disk, create a partition table */ EMPTY_FORCE, /* make disk empty, erase everything, create a partition table always */ EMPTY_CREATE, /* create disk as loopback file, create a partition table always */ -} arg_empty = EMPTY_UNSET; + _EMPTY_MODE_MAX, + _EMPTY_MODE_INVALID = -EINVAL, +} EmptyMode; typedef enum FilterPartitionType { FILTER_PARTITIONS_NONE, @@ -124,6 +126,7 @@ typedef enum FilterPartitionType { _FILTER_PARTITIONS_INVALID = -EINVAL, } FilterPartitionsType; +static EmptyMode arg_empty = EMPTY_UNSET; static bool arg_dry_run = true; static const char *arg_node = NULL; static char *arg_root = NULL; @@ -306,6 +309,15 @@ typedef struct Context { bool from_scratch; } Context; +static const char *empty_mode_table[_EMPTY_MODE_MAX] = { + [EMPTY_UNSET] = "unset", + [EMPTY_REFUSE] = "refuse", + [EMPTY_ALLOW] = "allow", + [EMPTY_REQUIRE] = "require", + [EMPTY_FORCE] = "force", + [EMPTY_CREATE] = "create", +}; + static const char *encrypt_mode_table[_ENCRYPT_MODE_MAX] = { [ENCRYPT_OFF] = "off", [ENCRYPT_KEY_FILE] = "key-file", @@ -326,6 +338,7 @@ static const char *minimize_mode_table[_MINIMIZE_MODE_MAX] = { [MINIMIZE_GUESS] = "guess", }; +DEFINE_PRIVATE_STRING_TABLE_LOOKUP(empty_mode, EmptyMode); DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN(encrypt_mode, EncryptMode, ENCRYPT_KEY_FILE); DEFINE_PRIVATE_STRING_TABLE_LOOKUP(verity_mode, VerityMode); DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN(minimize_mode, MinimizeMode, MINIMIZE_BEST); @@ -2431,7 +2444,7 @@ static int context_load_partition_table(Context *context) { from_scratch = true; break; - case EMPTY_UNSET: + default: assert_not_reached(); } @@ -6509,21 +6522,15 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_EMPTY: - if (isempty(optarg)) + if (isempty(optarg)) { arg_empty = EMPTY_UNSET; - else if (streq(optarg, "refuse")) - arg_empty = EMPTY_REFUSE; - else if (streq(optarg, "allow")) - arg_empty = EMPTY_ALLOW; - else if (streq(optarg, "require")) - arg_empty = EMPTY_REQUIRE; - else if (streq(optarg, "force")) - arg_empty = EMPTY_FORCE; - else if (streq(optarg, "create")) - arg_empty = EMPTY_CREATE; - else - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to parse --empty= parameter: %s", optarg); + break; + } + + arg_empty = empty_mode_from_string(optarg); + if (arg_empty < 0) + return log_error_errno(arg_empty, "Failed to parse --empty= parameter: %s", optarg); + break; case ARG_DISCARD: @@ -6857,10 +6864,7 @@ static int parse_argv(int argc, char *argv[]) { if (arg_definitions) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Combination of --make-ddi= and --definitions= is not supported."); if (!IN_SET(arg_empty, EMPTY_UNSET, EMPTY_CREATE)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Combination of --make-ddi= and --empty=%s is not supported.", - arg_empty == EMPTY_REFUSE ? "refuse" : - arg_empty == EMPTY_ALLOW ? "allow" : - arg_empty == EMPTY_REQUIRE ? "require" : "force"); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Combination of --make-ddi= and --empty=%s is not supported.", empty_mode_to_string(arg_empty)); /* Imply automatic sizing in DDI mode */ if (arg_size == UINT64_MAX)