From e66ee1a65133bfa64370d841d2b6de3e50ca376e Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 9 Feb 2021 17:10:54 +0100 Subject: [PATCH] BUG/MINOR: intops: fix mul32hi()'s off-by-one mul32hi() multiples a constant a with a variable b from 0 to 0xffffffff and shifts the result by 32 bits. It's visible that it's always impossible to reach the constant a this way because the product always misses exactly one unit of a to be preserved. And this cannot be corrected by the caller either as adding one to the output will only shift the output range, and it's not possible to pass 2^32 on the ratio . The right approach is to add "a" after the multiplication so that the input range is always preserved for all ratio values from 0 to 0xffffffff: (a=0x00000000 * b=0x00000000 + a=0x00000000) >> 32 = 0x00000000 (a=0x00000000 * b=0x00000001 + a=0x00000000) >> 32 = 0x00000000 (a=0x00000000 * b=0xffffffff + a=0x00000000) >> 32 = 0x00000000 (a=0x00000001 * b=0x00000000 + a=0x00000001) >> 32 = 0x00000000 (a=0x00000001 * b=0x00000001 + a=0x00000001) >> 32 = 0x00000000 (a=0x00000001 * b=0xffffffff + a=0x00000001) >> 32 = 0x00000001 (a=0xffffffff * b=0x00000000 + a=0xffffffff) >> 32 = 0x00000000 (a=0xffffffff * b=0x00000001 + a=0xffffffff) >> 32 = 0x00000001 (a=0xffffffff * b=0xffffffff + a=0xffffffff) >> 32 = 0xffffffff This is only used in freq_ctr calculations and the slightly lower value is unlikely to have ever been noticed by anyone. This may be backported though it is not important. --- include/haproxy/intops.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/haproxy/intops.h b/include/haproxy/intops.h index e5d795e63..a812874e5 100644 --- a/include/haproxy/intops.h +++ b/include/haproxy/intops.h @@ -55,7 +55,7 @@ void mask_prep_rank_map(unsigned long m, */ static inline unsigned int mul32hi(unsigned int a, unsigned int b) { - return ((unsigned long long)a * b) >> 32; + return ((unsigned long long)a * b + a) >> 32; } /* gcc does not know when it can safely divide 64 bits by 32 bits. Use this