]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
efivars: fix concurrent growth read accounting
authorLuca Boccassi <luca.boccassi@gmail.com>
Wed, 8 Jul 2026 12:07:44 +0000 (13:07 +0100)
committerLuca Boccassi <luca.boccassi@gmail.com>
Sat, 11 Jul 2026 16:07:52 +0000 (17:07 +0100)
efi_get_variable() allocates one byte for probing whether efivarfs
has grown since fstat(), then three more bytes for NUL termination.
Account for both sizes separately so a full readv() result is treated
as concurrent growth and retried before the terminators are written.

Follow-up for ab69a04600fd34c152c44be6864eb3bc64568e17

src/basic/efivars.c

index 0eeee1038efc09a2cb399d45636730e8a8c93d1d..b64b83664a8d72a0e20a79fb90b1c2af67d0c501 100644 (file)
@@ -87,14 +87,21 @@ int efi_get_variable(
                 }
 
                 /* We want +1 for the read call, and +3 for the additional terminating bytes added below. */
+                size_t file_size = (size_t) st.st_size, payload_size, read_size, alloc_size, read_limit;
+                if (!SUB_SAFE(&payload_size, file_size, sizeof(attr)) ||
+                    !ADD_SAFE(&read_size, payload_size, 1) ||
+                    !ADD_SAFE(&alloc_size, read_size, 3) ||
+                    !ADD_SAFE(&read_limit, file_size, 1))
+                        return log_debug_errno(SYNTHETIC_ERRNO(EOVERFLOW), "EFI variable '%s' size calculation overflow, refusing.", p);
+
                 free(buf);
-                buf = malloc((size_t) st.st_size - sizeof(attr) + CONST_MAX(1, 3));
+                buf = malloc(alloc_size);
                 if (!buf)
                         return -ENOMEM;
 
                 struct iovec iov[] = {
                         { &attr, sizeof(attr)                           },
-                        { buf,   (size_t) st.st_size - sizeof(attr) + 1 },
+                        { buf,   read_size                              },
                 };
 
                 n = readv(fd, iov, 2);
@@ -103,11 +110,11 @@ int efi_get_variable(
                                 return log_debug_errno(errno, "Reading from '%s' failed: %m", p);
 
                         log_debug("Reading from '%s' failed with EINTR, retrying.", p);
-                } else if ((size_t) n == sizeof(attr) + st.st_size + 1)
+                } else if ((size_t) n == read_limit)
                         /* We need to try again with a bigger buffer, the variable was apparently changed concurrently? */
                         log_debug("EFI variable '%s' larger than expected, retrying.", p);
                 else {
-                        assert((size_t) n < sizeof(attr) + st.st_size + 1);
+                        assert((size_t) n < read_limit);
                         break;
                 }