From 9ece644435c13ab01ce322b5c243fc2def3f4ec6 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 14 Aug 2020 18:50:46 +0100 Subject: [PATCH] core: change RootImageOptions to use names instead of partition numbers Follow the designations from the Discoverable Partitions Specification --- man/systemd.exec.xml | 50 +++++++++++++++++++++++++++++++++++--- src/core/dbus-execute.c | 26 +++++++++++++------- src/core/execute.c | 4 ++- src/core/load-fragment.c | 23 +++++++++--------- src/shared/bus-unit-util.c | 25 +++++++------------ src/shared/dissect-image.c | 12 ++++----- src/shared/dissect-image.h | 4 +-- test/units/testsuite-50.sh | 25 +++++++++++-------- 8 files changed, 110 insertions(+), 59 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index c339f3b885..c2e9cf2187 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -149,11 +149,53 @@ RootImageOptions= Takes a comma-separated list of mount options that will be used on disk images specified by - RootImage=. Optionally a partition number can be prefixed, followed by colon, in - case the image has multiple partitions, otherwise partition number 0 is implied. + RootImage=. Optionally a partition name can be prefixed, followed by colon, in + case the image has multiple partitions, otherwise partition name root is implied. Options for multiple partitions can be specified in a single line with space separators. Assigning an empty - string removes previous assignments. For a list of valid mount options, please refer to - mount8. + string removes previous assignments. Duplicated options are ignored. For a list of valid mount options, please + refer to mount8. + + Valid partition names follow the Discoverable + Partitions Specification. + + + Accepted partition names + + + + + + Partition Name + + + + + root + + + root-secondary + + + home + + + srv + + + esp + + + xbootldr + + + tmp + + + var + + + +
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index d5a24b9ab7..f611d677dd 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -802,12 +802,14 @@ static int property_get_root_image_options( assert(property); assert(reply); - r = sd_bus_message_open_container(reply, 'a', "(us)"); + r = sd_bus_message_open_container(reply, 'a', "(ss)"); if (r < 0) return r; LIST_FOREACH(mount_options, m, c->root_image_options) { - r = sd_bus_message_append(reply, "(us)", m->partition_number, m->options); + r = sd_bus_message_append(reply, "(ss)", + partition_designator_to_string(m->partition_designator), + m->options); if (r < 0) return r; } @@ -891,7 +893,7 @@ const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_PROPERTY("WorkingDirectory", "s", property_get_working_directory, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootDirectory", "s", NULL, offsetof(ExecContext, root_directory), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootImage", "s", NULL, offsetof(ExecContext, root_image), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("RootImageOptions", "a(us)", property_get_root_image_options, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("RootImageOptions", "a(ss)", property_get_root_image_options, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootHash", "ay", property_get_root_hash, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootHashPath", "s", NULL, offsetof(ExecContext, root_hash_path), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootHashSignature", "ay", property_get_root_hash_sig, 0, SD_BUS_VTABLE_PROPERTY_CONST), @@ -1371,29 +1373,35 @@ int bus_exec_context_set_transient_property( if (streq(name, "RootImageOptions")) { _cleanup_(mount_options_free_allp) MountOptions *options = NULL; _cleanup_free_ char *format_str = NULL; - const char *mount_options; - unsigned partition_number; + const char *mount_options, *partition; - r = sd_bus_message_enter_container(message, 'a', "(us)"); + r = sd_bus_message_enter_container(message, 'a', "(ss)"); if (r < 0) return r; - while ((r = sd_bus_message_read(message, "(us)", &partition_number, &mount_options)) > 0) { + while ((r = sd_bus_message_read(message, "(ss)", &partition, &mount_options)) > 0) { _cleanup_free_ char *previous = TAKE_PTR(format_str); _cleanup_free_ MountOptions *o = NULL; + int partition_designator; if (chars_intersect(mount_options, WHITESPACE)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid mount options string, contains whitespace character(s): %s", mount_options); - if (asprintf(&format_str, "%s%s%u:%s", strempty(previous), previous ? " " : "", partition_number, mount_options) < 0) + partition_designator = partition_designator_from_string(partition); + if (partition_designator < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid partition name: %s", partition); + + format_str = strjoin(previous, previous ? " " : "", partition, ":", mount_options); + if (!format_str) return -ENOMEM; o = new(MountOptions, 1); if (!o) return -ENOMEM; *o = (MountOptions) { - .partition_number = partition_number, + .partition_designator = partition_designator, .options = strdup(mount_options), }; if (!o->options) diff --git a/src/core/execute.c b/src/core/execute.c index 123396f6f0..ef8212fcb7 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4634,7 +4634,9 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { fprintf(f, "%sRootImageOptions:", prefix); LIST_FOREACH(mount_options, o, c->root_image_options) if (!isempty(o->options)) - fprintf(f, " %u:%s", o->partition_number, o->options); + fprintf(f, " %s:%s", + partition_designator_to_string(o->partition_designator), + o->options); fprintf(f, "\n"); } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index aed5674f2f..5da5f929cf 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1454,22 +1454,23 @@ int config_parse_root_image_options( } STRV_FOREACH_PAIR(first, second, l) { - _cleanup_free_ char *mount_options_resolved = NULL; - const char *mount_options = NULL; MountOptions *o = NULL; - unsigned int partition_number = 0; + _cleanup_free_ char *mount_options_resolved = NULL; + const char *mount_options = NULL, *partition = "root"; + int partition_designator; - /* Format is either '0:foo' or 'foo' (0 is implied) */ + /* Format is either 'root:foo' or 'foo' (root is implied) */ if (!isempty(*second)) { + partition = *first; mount_options = *second; - r = safe_atou(*first, &partition_number); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse partition number from \"%s\", ignoring: %m", *first); - continue; - } } else mount_options = *first; + partition_designator = partition_designator_from_string(partition); + if (partition_designator < 0) { + log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), "Invalid partition name %s, ignoring", partition); + continue; + } r = unit_full_printf(u, mount_options, &mount_options_resolved); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers in %s, ignoring: %m", mount_options); @@ -1480,10 +1481,10 @@ int config_parse_root_image_options( if (!o) return log_oom(); *o = (MountOptions) { - .partition_number = partition_number, + .partition_designator = partition_designator, .options = TAKE_PTR(mount_options_resolved), }; - LIST_APPEND(mount_options, options, o); + LIST_APPEND(mount_options, options, TAKE_PTR(o)); } /* empty spaces/separators only */ diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index d307e746f1..b5f98afa35 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -10,6 +10,7 @@ #include "condition.h" #include "coredump-util.h" #include "cpu-set-util.h" +#include "dissect-image.h" #include "escape.h" #include "exec-util.h" #include "exit-status.h" @@ -1468,11 +1469,11 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_open_container(m, 'v', "a(us)"); + r = sd_bus_message_open_container(m, 'v', "a(ss)"); if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_open_container(m, 'a', "(us)"); + r = sd_bus_message_open_container(m, 'a', "(ss)"); if (r < 0) return bus_log_create_error(r); @@ -1481,21 +1482,13 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con return log_error_errno(r, "Failed to parse argument: %m"); STRV_FOREACH_PAIR(first, second, l) { - const char *mount_options; - unsigned partition_number = 0; + /* Format is either 'root:foo' or 'foo' (root is implied) */ + if (!isempty(*second) && partition_designator_from_string(*first) < 0) + return bus_log_create_error(-EINVAL); - /* Format is either '0:foo' or 'foo' (0 is implied) */ - if (!isempty(*second)) { - mount_options = *second; - r = safe_atou(*first, &partition_number); - if (r < 0) { - log_error_errno(r, "Failed to parse partition number from %s: %m", *first); - continue; - } - } else - mount_options = *first; - - r = sd_bus_message_append(m, "(us)", partition_number, mount_options); + r = sd_bus_message_append(m, "(ss)", + !isempty(*second) ? *first : "root", + !isempty(*second) ? *second : *first); if (r < 0) return bus_log_create_error(r); } diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index f41d1a0e48..98cf15cd33 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -420,7 +420,7 @@ int dissect_image( m->verity = root_hash && verity_data; m->can_verity = !!verity_data; - options = mount_options_from_part(mount_options, 0); + options = mount_options_from_part(mount_options, PARTITION_ROOT); if (options) { o = strdup(options); if (!o) @@ -716,7 +716,7 @@ int dissect_image( if (!n) return -ENOMEM; - options = mount_options_from_part(mount_options, nr); + options = mount_options_from_part(mount_options, designator); if (options) { o = strdup(options); if (!o) @@ -773,7 +773,7 @@ int dissect_image( if (!n) return -ENOMEM; - options = mount_options_from_part(mount_options, nr); + options = mount_options_from_part(mount_options, PARTITION_XBOOTLDR); if (options) { o = strdup(options); if (!o) @@ -827,7 +827,7 @@ int dissect_image( if (multiple_generic) return -ENOTUNIQ; - options = mount_options_from_part(mount_options, generic_nr); + options = mount_options_from_part(mount_options, PARTITION_ROOT); if (options) { o = strdup(options); if (!o) @@ -2023,11 +2023,11 @@ MountOptions* mount_options_free_all(MountOptions *options) { return NULL; } -const char* mount_options_from_part(const MountOptions *options, unsigned int partition_number) { +const char* mount_options_from_part(const MountOptions *options, int designator) { MountOptions *m; LIST_FOREACH(mount_options, m, (MountOptions *)options) - if (partition_number == m->partition_number && !isempty(m->options)) + if (designator == m->partition_designator && !isempty(m->options)) return m->options; return NULL; diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h index 4d21789e18..96f9840400 100644 --- a/src/shared/dissect-image.h +++ b/src/shared/dissect-image.h @@ -87,14 +87,14 @@ struct DissectedImage { }; struct MountOptions { - unsigned partition_number; + int partition_designator; char *options; LIST_FIELDS(MountOptions, mount_options); }; MountOptions* mount_options_free_all(MountOptions *options); DEFINE_TRIVIAL_CLEANUP_FUNC(MountOptions*, mount_options_free_all); -const char* mount_options_from_part(const MountOptions *options, unsigned int partition_number); +const char* mount_options_from_part(const MountOptions *options, int designator); int probe_filesystem(const char *node, char **ret_fstype); int dissect_image(int fd, const void *root_hash, size_t root_hash_size, const char *verity_data, const MountOptions *mount_options, DissectImageFlags flags, DissectedImage **ret); diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh index bb3b20da3e..8a9cedf375 100755 --- a/test/units/testsuite-50.sh +++ b/test/units/testsuite-50.sh @@ -128,33 +128,38 @@ umount ${image_dir}/mount systemd-run -t --property RootImage=${image}.gpt --property RootHash=${roothash} /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property RootImage=${image}.raw --property RootImageOptions="1:ro,noatime 2:ro,dev nosuid,dev" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "nosuid" -systemd-run -t --property RootImage=${image}.gpt --property RootImageOptions="1:ro,noatime 1:ro,dev" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "noatime" +systemd-run -t --property RootImage=${image}.raw --property RootImageOptions="root:nosuid,dev home:ro,dev ro,noatime" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "nosuid" +systemd-run -t --property RootImage=${image}.gpt --property RootImageOptions="root:ro,noatime root:ro,dev" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "noatime" +mkdir -p mkdir -p ${image_dir}/result cat > /run/systemd/system/testservice-50a.service < /run/result/a" +BindPaths=${image_dir}/result:/run/result +TemporaryFileSystem=/run MountAPIVFS=yes RootImage=${image}.raw -RootImageOptions=1:ro,noatime,nosuid 2:ro,dev noatime,dev +RootImageOptions=root:ro,noatime home:ro,dev relatime,dev RootImageOptions=nosuid,dev EOF systemctl start testservice-50a.service -journalctl -b -u testservice-50a.service | grep -F "squashfs" | grep -q -F "noatime" -journalctl -b -u testservice-50a.service | grep -F "squashfs" | grep -q -F -v "nosuid" +grep -F "squashfs" ${image_dir}/result/a | grep -q -F "noatime" +grep -F "squashfs" ${image_dir}/result/a | grep -q -F -v "nosuid" cat > /run/systemd/system/testservice-50b.service < /run/result/b" +BindPaths=${image_dir}/result:/run/result +TemporaryFileSystem=/run MountAPIVFS=yes RootImage=${image}.gpt -RootImageOptions=1:ro,noatime,nosuid 2:ro,dev nosuid,dev -RootImageOptions=2:ro,dev nosuid,dev,%%foo +RootImageOptions=root:ro,noatime,nosuid home:ro,dev nosuid,dev +RootImageOptions=home:ro,dev nosuid,dev,%%foo EOF systemctl start testservice-50b.service -journalctl -b -u testservice-50b.service | grep -F "squashfs" | grep -q -F "noatime" +grep -F "squashfs" ${image_dir}/result/b | grep -q -F "noatime" # Check that specifier escape is applied %%foo -> %foo busctl get-property org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/testservice_2d50b_2eservice org.freedesktop.systemd1.Service RootImageOptions | grep -F "nosuid,dev,%foo"