From eabdd0252a2c3f8fb03b83781019243d47737e80 Mon Sep 17 00:00:00 2001 From: Yidi Lin Date: Thu, 2 Nov 2023 14:17:02 +0800 Subject: [PATCH] libpayload/libc/time: Fix possible overflow in multiplication The value from raw_read_cntfrq_el0() could be large enough to cause overflow when multiplied by USECS_PER_SEC. To prevent this, both USECS_PER_SEC and hz can be reduced by dividing them by their GCD. This patch also modifies the return type of `timer_hz()` from `uint64_t` to `uint32_t`, assuming that in practice the timestamp counter should never be that fast. BUG=b:307790895 TEST=boot to kernel and check the timestamps from `cbmem` Change-Id: Ia55532490651fcf47128b83a8554751f050bcc89 Signed-off-by: Yidi Lin Reviewed-on: https://review.coreboot.org/c/coreboot/+/78888 Reviewed-by: Julius Werner Reviewed-by: Yu-Ping Wu Tested-by: build bot (Jenkins) --- .../libpayload/drivers/timer/arm64_arch_timer.c | 2 +- payloads/libpayload/drivers/timer/generic.c | 2 +- payloads/libpayload/drivers/timer/rdtsc.c | 6 ++++-- payloads/libpayload/include/libpayload.h | 2 +- payloads/libpayload/libc/time.c | 13 +++++++++---- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/payloads/libpayload/drivers/timer/arm64_arch_timer.c b/payloads/libpayload/drivers/timer/arm64_arch_timer.c index 087d9e7565..b4d2b865ae 100644 --- a/payloads/libpayload/drivers/timer/arm64_arch_timer.c +++ b/payloads/libpayload/drivers/timer/arm64_arch_timer.c @@ -32,7 +32,7 @@ #include #include -uint64_t timer_hz(void) +uint32_t timer_hz(void) { return raw_read_cntfrq_el0(); } diff --git a/payloads/libpayload/drivers/timer/generic.c b/payloads/libpayload/drivers/timer/generic.c index ef9eda54ad..bd5674be8c 100644 --- a/payloads/libpayload/drivers/timer/generic.c +++ b/payloads/libpayload/drivers/timer/generic.c @@ -33,7 +33,7 @@ #include #include -uint64_t timer_hz(void) +uint32_t timer_hz(void) { /* libc/time.c currently requires all timers to be at least 1MHz. */ assert(CONFIG_LP_TIMER_GENERIC_HZ >= 1000000); diff --git a/payloads/libpayload/drivers/timer/rdtsc.c b/payloads/libpayload/drivers/timer/rdtsc.c index cfd56b064a..952bc0bb13 100644 --- a/payloads/libpayload/drivers/timer/rdtsc.c +++ b/payloads/libpayload/drivers/timer/rdtsc.c @@ -33,10 +33,12 @@ #include #include +#include -uint64_t timer_hz(void) +uint32_t timer_hz(void) { - return (uint64_t)lib_sysinfo.cpu_khz * 1000; + assert(UINT32_MAX / 1000 >= lib_sysinfo.cpu_khz); + return lib_sysinfo.cpu_khz * 1000; } uint64_t timer_raw_value(void) diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index e3c60ac189..06c6de429e 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -519,7 +519,7 @@ void lib_sysinfo_get_memranges(struct memrange **ranges, /* Timer functions. */ /* Defined by each architecture. */ -uint64_t timer_hz(void); +uint32_t timer_hz(void); uint64_t timer_raw_value(void); uint64_t timer_us(uint64_t base); /* Generic. */ diff --git a/payloads/libpayload/libc/time.c b/payloads/libpayload/libc/time.c index 6780008d4c..c38dbfdde8 100644 --- a/payloads/libpayload/libc/time.c +++ b/payloads/libpayload/libc/time.c @@ -38,6 +38,7 @@ #if CONFIG(LP_ARCH_X86) && CONFIG(LP_NVRAM) #include #endif +#include #include extern u32 cpu_khz; @@ -170,17 +171,21 @@ void arch_ndelay(uint64_t ns) u64 timer_us(u64 base) { - static u64 hz; + static u32 hz, mult = USECS_PER_SEC; + u32 div; // Only check timer_hz once. Assume it doesn't change. if (hz == 0) { hz = timer_hz(); - if (hz < 1000000) { - printf("Timer frequency %" PRIu64 " is too low, " + if (hz < mult) { + printf("Timer frequency %" PRIu32 " is too low, " "must be at least 1MHz.\n", hz); halt(); } + div = gcd32(hz, mult); + hz /= div; + mult /= div; } - return (1000000 * timer_raw_value()) / hz - base; + return (mult * timer_raw_value()) / hz - base; }