From 66600e15c913171f20f8da3f32467bf177395d62 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 9 Jul 2026 11:15:32 +0100 Subject: [PATCH 1/6] string-util: add upper bound to ellipsize_mem UTF-8 walks Validate UTF-8 characters against the caller-provided byte slice. This stops truncated sequences from borrowing bytes past old_length. Follow-up for e69cc9eb36fd6e76710b4d5f4bb7013980fb5174 --- src/basic/string-util.c | 59 ++++++++++++++++++++++++++++++++------- src/test/test-ellipsize.c | 6 ++++ 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 1dc90d538a..047a58ade5 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -357,6 +357,33 @@ static char *ascii_ellipsize_mem(const char *s, size_t old_length, size_t new_le return t; } +/* Walk backwards from 'end' (exclusive) to the start of the last complete UTF-8 character, without + * descending below 'start', validate it and return a pointer to its first byte, optionally decoding it + * into *ret_c. Returns NULL on a truncated or otherwise malformed sequence. */ +static const char* find_previous_unichar(const char *start, const char *end, char32_t *ret_c) { + const char *p; + int r; + + assert(start); + assert(end); + assert(end > start); + + for (p = end; p > start; ) { + p--; + if (((uint8_t) *p & 0xc0) != 0x80) /* Found a non-continuation byte, i.e. a character start. */ + break; + } + + r = utf8_encoded_valid_unichar(p, end - p); + if (r < 0 || p + r != end) + return NULL; + + if (ret_c) + assert_se(utf8_encoded_to_unichar(p, ret_c) == r); + + return p; +} + char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigned percent) { size_t x, k, len, len2; const char *i, *j; @@ -400,11 +427,13 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne continue; /* ANSI sequences don't take up any space in output */ } - char32_t c; - r = utf8_encoded_to_unichar(i, &c); + r = utf8_encoded_valid_unichar(i, s + old_length - i); if (r < 0) return NULL; + char32_t c; + assert_se(utf8_encoded_to_unichar(i, &c) == r); + int w = unichar_iswide(c) ? 2 : 1; if (k + w > x) break; @@ -431,9 +460,9 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne continue; } - tt = utf8_prev_char(t); - r = utf8_encoded_to_unichar(tt, &c); - if (r < 0) + /* Find the previous complete UTF-8 character inside the retained suffix. */ + tt = find_previous_unichar(i, t, &c); + if (!tt) return NULL; w = unichar_iswide(c) ? 2 : 1; @@ -451,11 +480,21 @@ char* ellipsize_mem(const char *s, size_t old_length, size_t new_length, unsigne if (k >= new_length) { /* Make space for ellipsis, if required and possible. We know that the edge character is not * part of an ANSI sequence (because then we'd skip it). If the last character we looked at - * was wide, we don't need to make space. */ - if (j < s + old_length) - j = utf8_next_char(j); - else if (i > s) - i = utf8_prev_char(i); + * was wide, we don't need to make space. + * Move the edge by one complete UTF-8 character within the input slice. */ + if (j < s + old_length) { + r = utf8_encoded_valid_unichar(j, s + old_length - j); + if (r < 0) + return NULL; + + j += r; + } else if (i > s) { + const char *tt = find_previous_unichar(s, i, NULL); + if (!tt) + return NULL; + + i = tt; + } } len = i - s; diff --git a/src/test/test-ellipsize.c b/src/test/test-ellipsize.c index 2efb123ac2..3d47cfebe2 100644 --- a/src/test/test-ellipsize.c +++ b/src/test/test-ellipsize.c @@ -75,6 +75,12 @@ TEST(ellipsize_mem) { test_ellipsize_mem_one(s, l, k); } +TEST(ellipsize_mem_truncated_utf8) { + static const char s[] = { 'a', '\xf0', '\x80', '\x80', '\x80', 'b' }; + + ASSERT_NULL(ellipsize_mem(s, 2, 1, 50)); +} + static void test_ellipsize_one(const char *p) { _cleanup_free_ char *t = NULL; t = ellipsize(p, columns(), 70); From f5e35f72cf8d930022cf9999664da3eb10251720 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 9 Jul 2026 11:30:02 +0100 Subject: [PATCH 2/6] log: add upper bound to journal iovec accounting Use checked arithmetic before clamping the journal iovec length. Stop copying input iovecs once the fixed journal vector is full. Follow-up for e69cc9eb36fd6e76710b4d5f4bb7013980fb5174 --- src/basic/log.c | 24 +++++++++++++++++++----- src/test/test-log.c | 22 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/basic/log.c b/src/basic/log.c index 29702abd44..17ea26d8a4 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -726,7 +726,11 @@ static int write_to_journal( if (LOG_PRI(level) > log_target_max_level[LOG_TARGET_JOURNAL]) return 0; - iovec_len = MIN(6 + log_context_num_fields() * 3, IOVEC_MAX); + if (!MUL_SAFE(&iovec_len, log_context_num_fields(), 3) || + !ADD_SAFE(&iovec_len, iovec_len, 6)) + iovec_len = IOVEC_MAX; + else + iovec_len = MIN(iovec_len, IOVEC_MAX); iovec = newa(struct iovec, iovec_len); log_do_header(header, sizeof(header), level, error, file, line, func, object_field, object, extra_field, extra); @@ -1024,7 +1028,11 @@ int log_struct_internal( int r; bool fallback = false; - iovec_len = MIN(17 + log_context_num_fields() * 3, IOVEC_MAX); + if (!MUL_SAFE(&iovec_len, log_context_num_fields(), 3) || + !ADD_SAFE(&iovec_len, iovec_len, 17)) + iovec_len = IOVEC_MAX; + else + iovec_len = MIN(iovec_len, IOVEC_MAX); iovec = newa(struct iovec, iovec_len); /* If the journal is available do structured logging. @@ -1123,15 +1131,21 @@ int log_struct_iovec_internal( char header[LINE_MAX]; struct iovec *iovec; - size_t n = 0, iovec_len; + size_t n = 0, iovec_len, n_context_iovec, n_input_iovec_expanded; - iovec_len = MIN(1 + n_input_iovec * 2 + log_context_num_fields() * 3, IOVEC_MAX); + if (!MUL_SAFE(&n_input_iovec_expanded, n_input_iovec, 2) || + !MUL_SAFE(&n_context_iovec, log_context_num_fields(), 3) || + !ADD_SAFE(&iovec_len, 1, n_input_iovec_expanded) || + !ADD_SAFE(&iovec_len, iovec_len, n_context_iovec)) + iovec_len = IOVEC_MAX; + else + iovec_len = MIN(iovec_len, IOVEC_MAX); iovec = newa(struct iovec, iovec_len); log_do_header(header, sizeof(header), level, error, file, line, func, NULL, NULL, NULL, NULL); iovec[n++] = IOVEC_MAKE_STRING(header); - for (size_t i = 0; i < n_input_iovec; i++) { + for (size_t i = 0; i < n_input_iovec && n + 2 <= iovec_len; i++) { iovec[n++] = input_iovec[i]; iovec[n++] = IOVEC_MAKE_STRING("\n"); } diff --git a/src/test/test-log.c b/src/test/test-log.c index fb23776b21..61468a3229 100644 --- a/src/test/test-log.c +++ b/src/test/test-log.c @@ -145,6 +145,28 @@ TEST(log_format_iovec) { LOG_ITEM("FOOFOO=%hu-%llu", (unsigned short) 4, (long long unsigned) 3)); } +TEST(log_struct_iovec_many_fields) { + LogTarget old_target = log_get_target(); + struct iovec iovec[1024]; + + log_set_target(LOG_TARGET_JOURNAL); + log_open(); + if (log_on_console()) { + log_set_target(old_target); + log_open(); + return (void) log_tests_skipped("journal socket is not available"); + } + + iovec[0] = IOVEC_MAKE_STRING("MESSAGE=many fields"); + for (size_t i = 1; i < ELEMENTSOF(iovec); i++) + iovec[i] = IOVEC_MAKE_STRING("FIELD=value"); + + ASSERT_OK(log_struct_iovec(LOG_INFO, iovec, ELEMENTSOF(iovec))); + + log_set_target(old_target); + log_open(); +} + static void test_log_struct(void) { log_struct(LOG_INFO, "MESSAGE=Waldo PID="PID_FMT" (no errno)", getpid_cached(), From ea44ee4e8a97bfd3737cc35cc6e4ed567828b8e4 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 9 Jul 2026 11:35:50 +0100 Subject: [PATCH 3/6] copy: avoid following fifo/node chmod target Use AT_SYMLINK_NOFOLLOW when applying copied FIFO and device node modes, matching the adjacent ownership and timestamp updates. Follow-up for e69cc9eb36fd6e76710b4d5f4bb7013980fb5174 --- src/shared/copy.c | 4 +-- src/test/test-copy.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/shared/copy.c b/src/shared/copy.c index 21add1d5c0..527022dcca 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -959,7 +959,7 @@ static int fd_copy_fifo( AT_SYMLINK_NOFOLLOW) < 0) r = -errno; - if (fchmodat(dt, to, st->st_mode & 07777, 0) < 0) + if (fchmodat(dt, to, st->st_mode & 07777, AT_SYMLINK_NOFOLLOW) < 0) r = -errno; (void) utimensat(dt, to, (struct timespec[]) { st->st_atim, st->st_mtim }, AT_SYMLINK_NOFOLLOW); @@ -1012,7 +1012,7 @@ static int fd_copy_node( AT_SYMLINK_NOFOLLOW) < 0) r = -errno; - if (fchmodat(dt, to, st->st_mode & 07777, 0) < 0) + if (fchmodat(dt, to, st->st_mode & 07777, AT_SYMLINK_NOFOLLOW) < 0) r = -errno; (void) utimensat(dt, to, (struct timespec[]) { st->st_atim, st->st_mtim }, AT_SYMLINK_NOFOLLOW); diff --git a/src/test/test-copy.c b/src/test/test-copy.c index 290a6bed8f..8465d6cc99 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -308,6 +309,69 @@ TEST(copy_tree_at_symlink) { fd = safe_close(fd); } +TEST(copy_tree_fifo_chmod_symlink_race) { + _cleanup_(rm_rf_physical_and_freep) char *srcp = NULL, *dstp = NULL; + _cleanup_close_ int src = -EBADF, dst = -EBADF; + _cleanup_close_pair_ int ready[2] = EBADF_PAIR, stop[2] = EBADF_PAIR; + struct stat st; + int status; + pid_t pid; + char c; + + if (!slow_tests_enabled()) + return (void) log_tests_skipped("slow tests are disabled"); + + ASSERT_OK(src = mkdtemp_open(NULL, 0, &srcp)); + ASSERT_OK(dst = mkdtemp_open(NULL, 0, &dstp)); + ASSERT_OK_ERRNO(mkfifoat(src, "fifo", 0777)); + ASSERT_OK_ERRNO(fchmodat(src, "fifo", 0777, 0)); + ASSERT_OK(write_string_file_at(dst, "victim", "victim", WRITE_STRING_FILE_CREATE)); + ASSERT_OK_ERRNO(fchmodat(dst, "victim", 0600, 0)); + + ASSERT_OK_ERRNO(pipe2(ready, O_CLOEXEC)); + ASSERT_OK_ERRNO(pipe2(stop, O_CLOEXEC|O_NONBLOCK)); + + pid = ASSERT_OK_ERRNO(fork()); + if (pid == 0) { + ready[0] = safe_close(ready[0]); + stop[1] = safe_close(stop[1]); + + assert_se(write(ready[1], &(const char) { 'x' }, 1) == 1); + + for (;;) { + char stop_char; + + ssize_t n = read(stop[0], &stop_char, 1); + if (n >= 0) + _exit(EXIT_SUCCESS); + + (void) unlinkat(dst, "fifo", 0); + (void) symlinkat("victim", dst, "fifo"); + } + } + + ready[1] = safe_close(ready[1]); + stop[0] = safe_close(stop[0]); + + ASSERT_OK_EQ_ERRNO(read(ready[0], &c, 1), 1); + + bool changed = false; + for (unsigned i = 0; i < 20000; i++) { + (void) copy_tree_at(src, "fifo", dst, "fifo", UID_INVALID, GID_INVALID, COPY_REPLACE, NULL, NULL); + + ASSERT_OK_ERRNO(fstatat(dst, "victim", &st, 0)); + if ((st.st_mode & 0777) != 0600) { + changed = true; + break; + } + } + + ASSERT_OK_EQ_ERRNO(write(stop[1], &(const char) { 'x' }, 1), 1); + ASSERT_OK_ERRNO(waitpid(pid, &status, 0)); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_FALSE(changed); +} + TEST_RET(copy_bytes) { _cleanup_close_pair_ int pipefd[2] = EBADF_PAIR; _cleanup_close_ int infd = -EBADF; From 39fa094b0d958b2d6c0dcd55377eb2b22370df48 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 9 Jul 2026 14:44:22 +0100 Subject: [PATCH 4/6] pull-oci: switch assert() to assert_se() for set_remove() call This has obvious side effects so switch it over Follow-up for a9f6ba04969d6eb2e629e30299fab7538ef42a57 --- src/import/pull-oci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/import/pull-oci.c b/src/import/pull-oci.c index c2d9bfeddf..5df55e4278 100644 --- a/src/import/pull-oci.c +++ b/src/import/pull-oci.c @@ -1293,7 +1293,7 @@ static void oci_pull_job_on_finished_layer(PullJob *j) { goto finish; } - assert(set_remove(i->active_layer_jobs, j) == j); + assert_se(set_remove(i->active_layer_jobs, j) == j); r = oci_pull_work(i); if (r <= 0) From 41fc062c491ebdc0e5ccce8da1669f31d48e970a Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 9 Jul 2026 15:07:54 +0100 Subject: [PATCH 5/6] homework-luks: add new key slots before destroying old ones home_passwd_luks() rotates the LUKS key slots with a single loop that destroys slot i before adding its replacement at the same index. If adding the replacement fails (e.g.: argon2 OOMs), slot i is left destroyed with no replacement and no rollback. If the user has only one password, its only key slot is now gone and the home directory can no longer be unlocked. Rotate in two passes instead: first add every new password into a free slot (CRYPT_ANY_SLOT), and only once all adds succeed, destroy the old slots. If an add fails, roll back the slots added so far and return with the pre-existing slots untouched, so at least one valid key slot always remains for each password the user holds. Follow-up for 70a5db5822c8056b53d9a4a9273ad12cb5f87a92 --- src/home/homework-luks.c | 89 ++++++++++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 18 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 7b9bd0d71a..b18805fa3e 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -3702,7 +3702,7 @@ int home_passwd_luks( const PasswordCache *cache, /* the passwords acquired via PKCS#11/FIDO2 security tokens */ char **effective_passwords /* new passwords */) { - size_t volume_key_size, max_key_slots, n_effective; + size_t volume_key_size, max_key_slots, n_effective, n_occupied = 0; _cleanup_(erase_and_freep) void *volume_key = NULL; struct crypt_pbkdf_type good_pbkdf, minimal_pbkdf; const char *type; @@ -3742,44 +3742,97 @@ int home_passwd_luks( n_effective = strv_length(effective_passwords); + for (size_t i = 0; i < max_key_slots; i++) + switch (sym_crypt_keyslot_status(setup->crypt_device, i)) { + case CRYPT_SLOT_INACTIVE: + break; + case CRYPT_SLOT_ACTIVE: + case CRYPT_SLOT_ACTIVE_LAST: + case CRYPT_SLOT_UNBOUND: + n_occupied++; + break; + default: + return log_error_errno(SYNTHETIC_ERRNO(EIO), + "Failed to determine state of LUKS key slot %zu.", i); + } + + if (n_effective > max_key_slots - n_occupied) + return log_error_errno(SYNTHETIC_ERRNO(ENOSPC), + "Not enough free LUKS key slots (%zu required, %zu available).", + n_effective, max_key_slots - n_occupied); + build_good_pbkdf(&good_pbkdf, h); build_minimal_pbkdf(&minimal_pbkdf, h); - for (size_t i = 0; i < max_key_slots; i++) { - r = sym_crypt_keyslot_destroy(setup->crypt_device, i); - if (r < 0 && !IN_SET(r, -ENOENT, -EINVAL)) /* Returns EINVAL or ENOENT if there's no key in this slot already */ - return log_error_errno(r, "Failed to destroy LUKS password: %m"); + /* Rotate the LUKS key slots in two passes, so that at every on-disk state at least one valid key slot + * exists for each password the user currently holds. First add all new passwords into free key slots + * (purely additive, hence non-destructive), and only once that fully succeeded destroy the old slots. + * If an add fails (e.g. the argon2 PBKDF, which is intentionally memory hungry, hits OOM), we roll + * back the slots added so far and return with the pre-existing slots untouched. The reverse order + * (destroy a slot first, then add its replacement at the same index) would, on such a failure, leave + * the slot destroyed with no replacement, permanently locking the user out. */ - if (i >= n_effective) { - if (r >= 0) - log_info("Destroyed LUKS key slot %zu.", i); - continue; - } + _cleanup_free_ int *new_slots = new(int, n_effective); + if (!new_slots) + return log_oom(); + size_t n_added = 0; + for (size_t i = 0; i < n_effective; i++) { if (password_cache_contains(cache, effective_passwords[i])) { /* Is this a FIDO2 or PKCS#11 password? */ - log_debug("Using minimal PBKDF for slot %zu", i); + log_debug("Using minimal PBKDF for new key slot."); r = sym_crypt_set_pbkdf_type(setup->crypt_device, &minimal_pbkdf); } else { - log_debug("Using good PBKDF for slot %zu", i); + log_debug("Using good PBKDF for new key slot."); r = sym_crypt_set_pbkdf_type(setup->crypt_device, &good_pbkdf); } - if (r < 0) - return log_error_errno(r, "Failed to tweak PBKDF for slot %zu: %m", i); + if (r < 0) { + log_error_errno(r, "Failed to tweak PBKDF for new key slot: %m"); + goto rollback; + } r = sym_crypt_keyslot_add_by_volume_key( setup->crypt_device, - i, + CRYPT_ANY_SLOT, volume_key, volume_key_size, effective_passwords[i], strlen(effective_passwords[i])); - if (r < 0) - return log_error_errno(r, "Failed to set up LUKS password: %m"); + if (r < 0) { + log_error_errno(r, "Failed to set up LUKS password: %m"); + goto rollback; + } - log_info("Updated LUKS key slot %zu.", i); + new_slots[n_added++] = r; + log_info("Added LUKS key slot %i.", r); + } + + /* All replacement slots are committed now; drop every slot that isn't one we just added. */ + for (size_t i = 0; i < max_key_slots; i++) { + bool keep = false; + + for (size_t j = 0; j < n_added; j++) + if (new_slots[j] == (int) i) { + keep = true; + break; + } + if (keep) + continue; + + r = sym_crypt_keyslot_destroy(setup->crypt_device, i); + if (r < 0 && !IN_SET(r, -ENOENT, -EINVAL)) /* Returns EINVAL or ENOENT if there's no key in this slot already */ + return log_error_errno(r, "Failed to destroy LUKS password: %m"); + if (r >= 0) + log_info("Destroyed LUKS key slot %zu.", i); } return 1; + +rollback: + /* Undo the slots added in this call, so that the header is left exactly as we found it. */ + for (size_t i = 0; i < n_added; i++) + (void) sym_crypt_keyslot_destroy(setup->crypt_device, new_slots[i]); + + return r; } int home_lock_luks(UserRecord *h, HomeSetup *setup) { From e6f6ced7295ffef4295ad81ec726c26aa632fd0f Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 9 Jul 2026 15:42:38 +0100 Subject: [PATCH 6/6] dissect-image: don't assert() on partition geometry from blkid The per-partition loop in dissect_image() reads the start and size (in 512-byte sectors) of each partition from libblkid and guards the following byte conversions with assert(). Images sizes are input, rather than programming, so return an error instead of asserting. Follow-up for 88b3300fdc64d5320fb50d0f369d3fc0885e15e8 --- src/shared/dissect-image.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index f9556ea6b4..af19a48eea 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1309,15 +1309,17 @@ static int dissect_image( start = sym_blkid_partition_get_start(pp); if (start < 0) return errno_or_else(EIO); - - assert((uint64_t) start < UINT64_MAX/512); + if ((uint64_t) start >= UINT64_MAX/512) + return log_debug_errno(SYNTHETIC_ERRNO(EOVERFLOW), + "Partition start LBA too large to convert to a byte offset, refusing."); errno = 0; size = sym_blkid_partition_get_size(pp); if (size < 0) return errno_or_else(EIO); - - assert((uint64_t) size < UINT64_MAX/512); + if ((uint64_t) size >= UINT64_MAX/512) + return log_debug_errno(SYNTHETIC_ERRNO(EOVERFLOW), + "Partition size in LBA too large to convert to a byte offset, refusing."); /* While probing we need the non-diskseq device node name to access the thing, hence mask off * DISSECT_IMAGE_DISKSEQ_DEVNODE. */