]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
random-seed: handle post-merge review nits
authorJason A. Donenfeld <Jason@zx2c4.com>
Wed, 16 Nov 2022 18:27:50 +0000 (19:27 +0100)
committerJason A. Donenfeld <Jason@zx2c4.com>
Thu, 17 Nov 2022 11:22:27 +0000 (12:22 +0100)
These are various misc things that came up after merging.

src/boot/bootctl.c
src/boot/efi/random-seed.c
src/boot/efi/util.h
src/random-seed/random-seed.c

index e04424b379389433329a34943fed00c6d1e0370b..afda914d5292d0f4313bc08c5048d52acdb8a192 100644 (file)
@@ -1975,7 +1975,7 @@ static int verb_list(int argc, char *argv[], void *userdata) {
 
 static int install_random_seed(const char *esp) {
         _cleanup_(unlink_and_freep) char *tmp = NULL;
-        unsigned char buffer[RANDOM_EFI_SEED_SIZE];
+        uint8_t buffer[RANDOM_EFI_SEED_SIZE];
         _cleanup_free_ char *path = NULL;
         _cleanup_close_ int fd = -1;
         size_t token_size;
index 04bfd526f8179914af81be40904459ae038a9b85..02f4dfbc7f3f7e45369b7577373a9283aa4f1355 100644 (file)
@@ -59,7 +59,6 @@ static EFI_STATUS acquire_system_token(void **ret, UINTN *ret_size) {
         assert(ret);
         assert(ret_size);
 
-        *ret_size = 0;
         err = efivar_get_raw(LOADER_GUID, L"LoaderSystemToken", &data, &size);
         if (err != EFI_SUCCESS) {
                 if (err != EFI_NOT_FOUND)
@@ -187,6 +186,7 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) {
         /* Get some system specific seed that the installer might have placed in an EFI variable. We include
          * it in our hash. This is protection against golden master image sloppiness, and it remains on the
          * system, even when disk images are duplicated or swapped out. */
+        size = 0;
         err = acquire_system_token(&system_token, &size);
         if (mode != RANDOM_SEED_ALWAYS && (err != EFI_SUCCESS || size < DESIRED_SEED_SIZE) && !seeded_by_efi)
                 return err;
@@ -246,6 +246,7 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) {
         size = sizeof(uefi_monotonic_counter);
         sha256_process_bytes(&size, sizeof(size), &hash);
         sha256_process_bytes(&uefi_monotonic_counter, size, &hash);
+
         err = RT->GetTime(&now, NULL);
         size = err == EFI_SUCCESS ? sizeof(now) : 0; /* Known to be flaky, so don't bark on error. */
         sha256_process_bytes(&size, sizeof(size), &hash);
@@ -295,7 +296,8 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) {
         if (err != EFI_SUCCESS)
                 return log_error_status_stall(err, L"Failed to flush random seed file: %r", err);
 
-        err = BS->AllocatePool(EfiACPIReclaimMemory, sizeof(*new_seed_table) + DESIRED_SEED_SIZE,
+        err = BS->AllocatePool(EfiACPIReclaimMemory,
+                               offsetof(struct linux_efi_random_seed, seed) + DESIRED_SEED_SIZE,
                                (void **) &new_seed_table);
         if (err != EFI_SUCCESS)
                 return log_error_status_stall(err, L"Failed to allocate EFI table for random seed: %r", err);
index 15dd87f7747cc7231f9eebf50174989037bfd801..eed28c0342ca1961e33b2ad69952c1aa92818000 100644 (file)
@@ -20,7 +20,7 @@ __attribute__((noreturn)) extern void __assert_cl_failure__(void) __attribute__(
 __attribute__((noreturn)) extern void __assert_cl_failure__(void);
 #endif
 /* assert_cl generates a later-stage compile-time assertion when constant folding occurs. */
-#define assert_cl(condition) if (!(condition)) __assert_cl_failure__()
+#define assert_cl(condition) ({ if (!(condition)) __assert_cl_failure__(); })
 #else
 #define assert_cl(condition) assert(condition)
 #endif
@@ -60,7 +60,7 @@ static inline void freep(void *p) {
 
 static __always_inline void erase_obj(void *p) {
         size_t l;
-        assert_cl(p != NULL);
+        assert_cl(p);
         l = __builtin_object_size(p, 0);
         assert_cl(l != (size_t) -1);
         explicit_bzero_safe(p, l);
index ccb8e47e19abb6ac08ee1d186e2ee8956f60fafc..020840e0dfdaf84173133dfae98375ec1a8b61f0 100644 (file)
@@ -320,9 +320,10 @@ static int refresh_boot_seed(void) {
         struct sha256_ctx hash_state;
         _cleanup_free_ void *seed_file_bytes = NULL;
         _cleanup_free_ char *esp_path = NULL;
-        _cleanup_close_ int seed_fd = -1;
+        _cleanup_close_ int seed_fd = -1, dir_fd = -1;
         size_t len;
-        ssize_t r;
+        ssize_t n;
+        int r;
 
         assert_cc(RANDOM_EFI_SEED_SIZE == SHA256_DIGEST_SIZE);
 
@@ -336,48 +337,48 @@ static int refresh_boot_seed(void) {
                 return r; /* find_esp_and_warn() already logged */
         }
 
-        seed_fd = chase_symlinks_and_open("/loader/random-seed", esp_path,
-                                          CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS,
-                                          O_RDWR|O_CLOEXEC|O_NOCTTY, NULL);
-        if (seed_fd == -ENOENT) {
+        r = chase_symlinks("/loader", esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, NULL, &dir_fd);
+        if (r < 0) {
+                if (r == -ENOENT) {
+                        log_debug_errno(r, "Couldn't find ESP loader directory, so not updating ESP random seed.");
+                        return 0;
+                }
+                return log_error_errno(r, "Failed to open ESP loader directory: %m");
+        }
+        seed_fd = openat(dir_fd, "random-seed", O_NOFOLLOW|O_RDWR|O_CLOEXEC|O_NOCTTY);
+        if (seed_fd < 0 && errno == ENOENT) {
                 uint64_t features;
-
                 r = efi_loader_get_features(&features);
-                if (r == 0 && FLAGS_SET(features, EFI_LOADER_FEATURE_RANDOM_SEED)) {
-                        int dir_fd = chase_symlinks_and_open("/loader", esp_path,
-                                                             CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS,
-                                                             O_DIRECTORY|O_CLOEXEC|O_NOCTTY, NULL);
-                        if (dir_fd >= 0) {
-                                seed_fd = openat(dir_fd, "random-seed", O_CREAT|O_EXCL|O_RDWR|O_CLOEXEC|O_NOCTTY, 0600);
-                                close(dir_fd);
-                        }
+                if (r == 0 && FLAGS_SET(features, EFI_LOADER_FEATURE_RANDOM_SEED))
+                        seed_fd = openat(dir_fd, "random-seed", O_CREAT|O_EXCL|O_RDWR|O_CLOEXEC|O_NOCTTY, 0600);
+                else {
+                        log_debug_errno(seed_fd, "Couldn't find ESP random seed, and not booted with systemd-boot, so not updating ESP random seed.");
+                        return 0;
                 }
         }
-        if (seed_fd < 0) {
-                log_debug_errno(seed_fd, "Failed to open EFI seed path: %m");
-                return 0;
-        }
+        if (seed_fd < 0)
+                return log_error_errno(errno, "Failed to open EFI seed path: %m");
         r = random_seed_size(seed_fd, &len);
         if (r < 0)
                 return log_error_errno(r, "Failed to determine EFI seed path length: %m");
         seed_file_bytes = malloc(len);
         if (!seed_file_bytes)
                 return log_oom();
-        r = loop_read(seed_fd, seed_file_bytes, len, false);
-        if (r < 0)
-                return log_error_errno(r, "Failed to read EFI seed file: %m");
+        n = loop_read(seed_fd, seed_file_bytes, len, false);
+        if (n < 0)
+                return log_error_errno(n, "Failed to read EFI seed file: %m");
 
         /* Hash the old seed in so that we never regress in entropy. */
         sha256_init_ctx(&hash_state);
-        sha256_process_bytes(&r, sizeof(r), &hash_state);
-        sha256_process_bytes(seed_file_bytes, r, &hash_state);
+        sha256_process_bytes(&n, sizeof(n), &hash_state);
+        sha256_process_bytes(seed_file_bytes, n, &hash_state);
 
         /* We're doing this opportunistically, so if the seeding dance before didn't manage to initialize the
          * RNG, there's no point in doing it here. Secondly, getrandom(GRND_NONBLOCK) has been around longer
          * than EFI seeding anyway, so there's no point in having non-getrandom() fallbacks here. So if this
          * fails, just return early to cut our losses. */
-        r = getrandom(buffer, sizeof(buffer), GRND_NONBLOCK);
-        if (r < 0) {
+        n = getrandom(buffer, sizeof(buffer), GRND_NONBLOCK);
+        if (n < 0) {
                 if (errno == EAGAIN) {
                         log_debug_errno(errno, "Random pool not initialized yet, so skipping EFI seed update");
                         return 0;
@@ -388,11 +389,11 @@ static int refresh_boot_seed(void) {
                 }
                 return log_error_errno(errno, "Failed to generate random bytes for EFI seed: %m");
         }
-        assert(r == sizeof(buffer));
+        assert(n == sizeof(buffer));
 
         /* Hash the new seed into the state containing the old one to generate our final seed. */
-        sha256_process_bytes(&r, sizeof(r), &hash_state);
-        sha256_process_bytes(buffer, r, &hash_state);
+        sha256_process_bytes(&n, sizeof(n), &hash_state);
+        sha256_process_bytes(buffer, n, &hash_state);
         sha256_finish_ctx(&hash_state, buffer);
 
         if (lseek(seed_fd, 0, SEEK_SET) < 0)
@@ -404,7 +405,7 @@ static int refresh_boot_seed(void) {
                 return log_error_errno(errno, "Failed to truncate EFI seed file: %m");
         r = fsync_full(seed_fd);
         if (r < 0)
-                return log_error_errno(errno, "Failed to fsync EFI seed file: %m");
+                return log_error_errno(r, "Failed to fsync EFI seed file: %m");
 
         log_debug("Updated random seed in ESP");
         return 0;