diff --git a/src/basic/efivars.c b/src/basic/efivars.c index b5e12eda62..f54c3d22e9 100644 --- a/src/basic/efivars.c +++ b/src/basic/efivars.c @@ -14,6 +14,7 @@ #include "io-util.h" #include "log.h" #include "memory-util.h" +#include "stat-util.h" #include "string-util.h" #include "time-util.h" #include "utf8.h" @@ -32,6 +33,7 @@ int efi_get_variable( void **ret_value, size_t *ret_size) { + int r; usec_t begin = 0; /* Unnecessary initialization to appease gcc */ assert(variable); @@ -66,13 +68,16 @@ 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); + r = stat_verify_regular(&st); + if (r < 0) + return log_debug_errno(r, "EFI variable '%s' is not a regular file, refusing: %m", 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. */ @@ -81,31 +86,37 @@ int efi_get_variable( } /* We want +1 for the read call, and +3 for the additional terminating bytes added below. */ - char *t = realloc(buf, (size_t) st.st_size + MAX(1, 3)); - if (!t) + free(buf); + buf = malloc((size_t) st.st_size - sizeof(attr) + CONST_MAX(1, 3)); + if (!buf) return -ENOMEM; - buf = t; - 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); + + /* Start from the beginning */ + (void) lseek(fd, 0, SEEK_SET); } /* Unfortunately kernel reports EOF if there's an inconsistency between efivarfs var list and @@ -122,19 +133,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 +162,7 @@ int efi_get_variable( * with a smaller value. */ if (ret_size) - *ret_size = n - 4; + *ret_size = value_size; return 0; } diff --git a/src/home/homed-home.c b/src/home/homed-home.c index a359baea59..e4f3487473 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -570,7 +570,7 @@ static int home_parse_worker_stdout(int _fd, UserRecord **ret) { return 0; } - if (lseek(fd, SEEK_SET, 0) < 0) + if (lseek(fd, 0, SEEK_SET) < 0) return log_error_errno(errno, "Failed to seek to beginning of memfd: %m"); f = take_fdopen(&fd, "r"); diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 80ef38f50a..01a810a9b8 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -365,7 +365,7 @@ static int raw_pull_make_local_copy(RawPull *i) { assert(i->raw_job->disk_fd >= 0); assert(i->offset == UINT64_MAX); - if (lseek(i->raw_job->disk_fd, SEEK_SET, 0) < 0) + if (lseek(i->raw_job->disk_fd, 0, SEEK_SET) < 0) return log_error_errno(errno, "Failed to seek to beginning of vendor image: %m"); } diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index d30cc6974f..3a3148be14 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -570,8 +570,8 @@ int seat_read_active_vt(Seat *s) { if (!seat_has_vts(s)) return 0; - if (lseek(s->manager->console_active_fd, SEEK_SET, 0) < 0) - return log_error_errno(errno, "lseek on console_active_fd failed: %m"); + if (lseek(s->manager->console_active_fd, 0, SEEK_SET) < 0) + return log_error_errno(errno, "lseek() on console_active_fd failed: %m"); errno = 0; k = read(s->manager->console_active_fd, t, sizeof(t)-1); diff --git a/src/test/test-copy.c b/src/test/test-copy.c index 86de5c4b63..58797b5926 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -124,7 +124,7 @@ TEST(copy_file_fd) { assert_se(write_string_file(in_fn, text, WRITE_STRING_FILE_CREATE) == 0); assert_se(copy_file_fd("/a/file/which/does/not/exist/i/guess", out_fd, COPY_REFLINK) < 0); assert_se(copy_file_fd(in_fn, out_fd, COPY_REFLINK) >= 0); - assert_se(lseek(out_fd, SEEK_SET, 0) == 0); + assert_se(lseek(out_fd, 0, SEEK_SET) == 0); assert_se(read(out_fd, buf, sizeof buf) == (ssize_t) strlen(text)); ASSERT_STREQ(buf, text); diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index 5723f2a074..b797845334 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -525,7 +525,7 @@ TEST(pid_get_cmdline_harder) { #define EXPECT1p "foo $'\\'bar\\'' $'\"bar$\"' $'x y z' $'!``'" #define EXPECT1v STRV_MAKE("foo", "'bar'", "\"bar$\"", "x y z", "!``") - ASSERT_OK_ZERO_ERRNO(lseek(fd, SEEK_SET, 0)); + ASSERT_OK_ZERO_ERRNO(lseek(fd, 0, SEEK_SET)); ASSERT_OK_EQ_ERRNO(write(fd, CMDLINE1, sizeof(CMDLINE1)), (ssize_t) sizeof(CMDLINE1)); ASSERT_OK_ZERO_ERRNO(ftruncate(fd, sizeof(CMDLINE1))); @@ -550,7 +550,7 @@ TEST(pid_get_cmdline_harder) { #define EXPECT2p "foo $'\\001\\002\\003'" #define EXPECT2v STRV_MAKE("foo", "\1\2\3") - ASSERT_OK_ZERO_ERRNO(lseek(fd, SEEK_SET, 0)); + ASSERT_OK_ZERO_ERRNO(lseek(fd, 0, SEEK_SET)); ASSERT_OK_EQ_ERRNO(write(fd, CMDLINE2, sizeof(CMDLINE2)), (ssize_t) sizeof(CMDLINE2)); ASSERT_OK_ZERO_ERRNO(ftruncate(fd, sizeof CMDLINE2)); diff --git a/src/udev/mtd_probe/probe_smartmedia.c b/src/udev/mtd_probe/probe_smartmedia.c index 9ce49b8552..e2c325ac74 100644 --- a/src/udev/mtd_probe/probe_smartmedia.c +++ b/src/udev/mtd_probe/probe_smartmedia.c @@ -69,7 +69,7 @@ int probe_smart_media(int mtd_fd, mtd_info_t* info) { } for (offset = 0; offset < block_size * spare_count; offset += sector_size) { - (void) lseek(mtd_fd, SEEK_SET, offset); + (void) lseek(mtd_fd, offset, SEEK_SET); if (read(mtd_fd, cis_buffer, SM_SECTOR_SIZE) == SM_SECTOR_SIZE) { cis_found = 1;