morgan/systemd
1
0
Fork 0
mirror of https://github.com/morgan9e/systemd synced 2026-04-14 16:37:19 +09:00
Code Issues Packages Projects Releases Wiki Activity

efivars: fix size checks in efi_get_variable()

Browse Source 
writev() returns the full size, not just the payload size, hence always
add sizeof(attr) where necessary.

Let's also change a couple of "4" into sizeof(attr) all over the place,
to make clear what they are about.

Fixes: #39695
Follow-up for: 9db9d6806e
This commit is contained in:
Lennart Poettering
2025-11-13 12:14:34 +01:00
parent 88e26303ce
commit ab69a04600
1 changed files with 31 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
src/basic/efivars.c
View File

@@ -66,13 +66,12 @@ int efi_get_variable(
if (fstat(fd, &st) < 0)
return log_debug_errno(errno, "fstat(\"%s\") failed: %m", p);
if (st.st_size == 0)
return log_debug_errno(SYNTHETIC_ERRNO(ENOENT),
"EFI variable %s is uncommitted", p);
if (st.st_size < 4)
return log_debug_errno(SYNTHETIC_ERRNO(ENODATA), "EFI variable %s is shorter than 4 bytes, refusing.", p);
if (st.st_size > 4*1024*1024 + 4)
return log_debug_errno(SYNTHETIC_ERRNO(E2BIG), "EFI variable %s is ridiculously large, refusing.", p);
if (st.st_size == 0) /* for uncommited variables, see below */
return log_debug_errno(SYNTHETIC_ERRNO(ENOENT), "EFI variable '%s' is uncommitted", p);
if ((uint64_t) st.st_size < sizeof(attr))
return log_debug_errno(SYNTHETIC_ERRNO(ENODATA), "EFI variable '%s' is shorter than %zu bytes, refusing.", p, sizeof(attr));
if ((uint64_t) st.st_size > sizeof(attr) + 4 * U64_MB)
return log_debug_errno(SYNTHETIC_ERRNO(E2BIG), "EFI variable '%s' is ridiculously large, refusing.", p);
if (!ret_attribute && !ret_value) {
/* No need to read anything, return the reported size. */
@@ -82,28 +81,31 @@ int efi_get_variable(
/* We want +1 for the read call, and +3 for the additional terminating bytes added below. */
free(buf);
buf = malloc((size_t) st.st_size + MAX(1, 3));
buf = malloc((size_t) st.st_size - sizeof(attr) + CONST_MAX(1, 3));
if (!buf)
return -ENOMEM;
const struct iovec iov[] = {
{ &attr, sizeof(attr) },
{ buf, (size_t) st.st_size + 1 },
struct iovec iov[] = {
{ &attr, sizeof(attr) },
{ buf, (size_t) st.st_size - sizeof(attr) + 1 },
};
n = readv(fd, iov, 2);
assert(n <= st.st_size + 1);
if (n == st.st_size + 1)
/* We need to try again with a bigger buffer. */
continue;
if (n >= 0)
break;
if (n < 0) {
if (errno != EINTR)
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)
/* 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);
break;
}
log_debug_errno(errno, "Reading from \"%s\" failed: %m", p);
if (errno != EINTR)
return -errno;
if (try >= EFI_N_RETRIES_TOTAL)
return -EBUSY;
return log_debug_errno(SYNTHETIC_ERRNO(EBUSY), "Reading EFI variable '%s' failed even after %u tries, giving up.", p, try);
if (try >= EFI_N_RETRIES_NO_DELAY)
(void) usleep_safe(EFI_RETRY_DELAY);
}
@@ -122,19 +124,21 @@ int efi_get_variable(
if (n == 0)
return log_debug_errno(SYNTHETIC_ERRNO(ENOENT),
"EFI variable %s is uncommitted", p);
if (n < 4)
if ((size_t) n < sizeof(attr))
return log_debug_errno(SYNTHETIC_ERRNO(EIO),
"Read %zi bytes from EFI variable %s, expected >= 4", n, p);
"Read %zi bytes from EFI variable %s, expected >= %zu", n, p, sizeof(attr));
size_t value_size = n - sizeof(attr);
if (ret_attribute)
*ret_attribute = attr;
if (ret_value) {
assert(buf);
/* Always NUL-terminate (3 bytes, to properly protect UTF-16, even if truncated in
* the middle of a character) */
buf[n - 4] = 0;
buf[n - 4 + 1] = 0;
buf[n - 4 + 2] = 0;
buf[value_size] = 0;
buf[value_size + 1] = 0;
buf[value_size + 2] = 0;
*ret_value = TAKE_PTR(buf);
}
@@ -149,7 +153,7 @@ int efi_get_variable(
* with a smaller value. */
if (ret_size)
*ret_size = n - 4;
*ret_size = value_size;
return 0;
}