From 3ee413e632335d90d6accbbc3a86b8e42d61fc36 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 4 Dec 2023 18:18:48 +0100 Subject: [PATCH 1/5] dissect-image: path[] is now defined for all metadata fields There's no point in handling a case that never can happen. --- src/shared/dissect-image.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 2687eafaf6..b63973cea8 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -3390,10 +3390,7 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ assert(m); for (; n_meta_initialized < _META_MAX; n_meta_initialized ++) { - if (!paths[n_meta_initialized]) { - fds[2*n_meta_initialized] = fds[2*n_meta_initialized+1] = -EBADF; - continue; - } + assert(paths[n_meta_initialized]); if (pipe2(fds + 2*n_meta_initialized, O_CLOEXEC) < 0) { r = -errno; @@ -3435,8 +3432,7 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ for (unsigned k = 0; k < _META_MAX; k++) { _cleanup_close_ int fd = -ENOENT; - if (!paths[k]) - continue; + assert(paths[k]); fds[2*k] = safe_close(fds[2*k]); @@ -3535,8 +3531,7 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ for (unsigned k = 0; k < _META_MAX; k++) { _cleanup_fclose_ FILE *f = NULL; - if (!paths[k]) - continue; + assert(paths[k]); fds[2*k+1] = safe_close(fds[2*k+1]); From 29b4db7ede36ecba27b417e8d3e834ebacd36e89 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 4 Dec 2023 18:19:27 +0100 Subject: [PATCH 2/5] dissect-image: handle 'continue' event in metadata acquisition uniformly Let's jump to the same label in all cases, that closes the associated pipe, systematically. --- src/shared/dissect-image.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index b63973cea8..7142429642 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -3494,7 +3494,7 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ if (r < 0) goto inner_fail; - continue; + goto next; } default: @@ -3507,14 +3507,14 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ if (fd < 0) { log_debug_errno(fd, "Failed to read %s file of image, ignoring: %m", paths[k]); - fds[2*k+1] = safe_close(fds[2*k+1]); - continue; + goto next; } r = copy_bytes(fd, fds[2*k+1], UINT64_MAX, 0); if (r < 0) goto inner_fail; + next: fds[2*k+1] = safe_close(fds[2*k+1]); } From a8e8bcfb7dbe53158c30b236e66814b32a6b748b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 4 Dec 2023 18:20:36 +0100 Subject: [PATCH 3/5] dissect-image: don't try to validate an extension release file with no image name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise we might validate the OS release file instead… --- src/shared/dissect-image.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 7142429642..65f5e786b5 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -2179,7 +2179,7 @@ int dissected_image_mount( if (r > 0) ok = true; } - if (!ok && FLAGS_SET(flags, DISSECT_IMAGE_VALIDATE_OS_EXT)) { + if (!ok && FLAGS_SET(flags, DISSECT_IMAGE_VALIDATE_OS_EXT) && m->image_name) { r = extension_has_forbidden_content(where); if (r < 0) return r; @@ -3439,6 +3439,9 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ switch (k) { case META_SYSEXT_RELEASE: + if (!m->image_name) + goto next; + /* As per the os-release spec, if the image is an extension it will have a * file named after the image name in extension-release.d/ - we use the image * name and try to resolve it with the extension-release helpers, as @@ -3459,6 +3462,9 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ break; case META_CONFEXT_RELEASE: + if (!m->image_name) + goto next; + /* As above */ r = open_extension_release( t, From 8d5e61db432932faa5b2d8531ab804bb4da4791d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 4 Dec 2023 18:21:23 +0100 Subject: [PATCH 4/5] dissect-image: fix fd leak in dissected_image_acquire_metadata() We have to go through the "finish" label to properly close all pipes in the error path, so that we don't leak them. --- src/shared/dissect-image.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 65f5e786b5..62aca296f6 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -3629,18 +3629,25 @@ int dissected_image_acquire_metadata(DissectedImage *m, DissectImageFlags extra_ r = wait_for_terminate_and_check("(sd-dissect)", child, 0); child = 0; if (r < 0) - return r; + goto finish; n = read(error_pipe[0], &v, sizeof(v)); - if (n < 0) - return -errno; - if (n == sizeof(v)) - return v; /* propagate error sent to us from child */ - if (n != 0) - return -EIO; - - if (r != EXIT_SUCCESS) - return -EPROTO; + if (n < 0) { + r = -errno; + goto finish; + } + if (n == sizeof(v)) { + r = v; /* propagate error sent to us from child */ + goto finish; + } + if (n != 0) { + r = -EIO; + goto finish; + } + if (r != EXIT_SUCCESS) { + r = -EPROTO; + goto finish; + } free_and_replace(m->hostname, hostname); m->machine_id = machine_id; From 3f8229fbb069c39d2082a0f49f07ddbce61c7b75 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 4 Dec 2023 18:28:45 +0100 Subject: [PATCH 5/5] dissect-image: move comment to right place The image name is extracted from the image path originally passed in, i.e. not the contents of the image. And the image UUID is directly retrieved from the partition table, hence also not from the contents. Let's hence move the comment to separate out the stuff extract from the file systems (and thus only available when mounting/with privs/with block devices) from the data available without any of that. --- src/shared/dissect-image.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h index 979fd384fe..15c0bf7219 100644 --- a/src/shared/dissect-image.h +++ b/src/shared/dissect-image.h @@ -103,9 +103,10 @@ struct DissectedImage { uint32_t sector_size; - /* Meta information extracted from /etc/os-release and similar */ char *image_name; sd_id128_t image_uuid; + + /* Meta information extracted from /etc/os-release and similar */ char *hostname; sd_id128_t machine_id; char **machine_info;