From: Sitsofe Wheeler Date: Tue, 22 Aug 2017 05:19:29 +0000 (+0100) Subject: time: fix overflow in timespec_add_msec X-Git-Tag: fio-3.1~43^2 X-Git-Url: https://git.kernel.dk/?p=fio.git;a=commitdiff_plain;h=90e2cd1e1cfc6c2bb4c5643b4864347445f9571e time: fix overflow in timespec_add_msec Commit 8b6a404cdd2c40715885e562416c3db039912773 ("nanosecond: initial commit changing timeval to timespec") introduced a change to timespec_add_msec that decreased the time to (unsigned) overflow of adj_nsec leading to strange values in the write_*_log files when using log_avg_msec. The following fio job reproduces the problem: ./fio --ioengine=null --size=2G --time_based --runtime=11s \ --ramp_time=5s --write_iops_log=overflow --log_avg_msec=5000 \ --name=overflow cat overflow_iops.1.log shows the following: 5000, 997853, 0, 0 10640, 141323, 0, 0 Compiling fio with clang's undefined behaviour sanitizer with unsigned wraparound detection turned on by using CC=clang ./configure --extra-cflags="-fsanitize=undefined,integer" and running the previous job produces a lot of false positives but also flags up this: time.c:11:35: runtime error: unsigned integer overflow: 1000000 * 5000 cannot be represented in type 'unsigned int' Fix this by forcing the type of the integer literal to ensure that a 64 bit multiplication takes place. Further, force the result variables to be uint64_t allowing us to remove UL from other literals. Finally adjust some spacing while we're here. Fixes https://github.com/axboe/fio/issues/426 (which was also reported on the mailing list - http://www.spinics.net/lists/fio/msg06190.html ). Reported-by: Joseph R Gruher Suggested-by: Vincent Fu Suggested-by: Jeff Furlong Reviewed-by: Vincent Fu Tested-by: Joseph R Gruher Signed-off-by: Sitsofe Wheeler --- diff --git a/time.c b/time.c index edfe779b..07984190 100644 --- a/time.c +++ b/time.c @@ -8,17 +8,17 @@ static unsigned long ns_granularity; void timespec_add_msec(struct timespec *ts, unsigned int msec) { - unsigned long adj_nsec = 1000000 * msec; + uint64_t adj_nsec = 1000000ULL * msec; ts->tv_nsec += adj_nsec; if (adj_nsec >= 1000000000) { - unsigned long adj_sec = adj_nsec / 1000000000UL; + uint64_t adj_sec = adj_nsec / 1000000000; - ts->tv_nsec -= adj_sec * 1000000000UL; + ts->tv_nsec -= adj_sec * 1000000000; ts->tv_sec += adj_sec; } - if (ts->tv_nsec >= 1000000000UL){ - ts->tv_nsec -= 1000000000UL; + if (ts->tv_nsec >= 1000000000){ + ts->tv_nsec -= 1000000000; ts->tv_sec++; } }