From 59bf1844db0e96af1d53448e41635a2a972c4a03 Mon Sep 17 00:00:00 2001 From: Kuan-Wei Chiu Date: Wed, 25 Dec 2024 00:11:31 +0800 Subject: [PATCH] Fix undefined behavior in qsort comparison functions for rv_histogram The freq variable is of type size_t, but the qsort comparison functions were directly returning a->freq - b->freq, which implicitly converts the result to an int. This conversion can cause overflow, leading to implementation-defined behavior. When freq values are sufficiently large, this issue may violate the antisymmetric and transitive properties required for comparison functions: Antisymmetry: If a < b, then b > a. Transitivity: If a < b and b < c, then a < c. Violating these properties results in undefined behavior in qsort, which could trigger memory corruption in some glibc implementations, posing a potential security risk. [1] Rewrite the comparison functions (cmp_dec and cmp_asc) to compare size_t values explicitly, ensuring correctness and avoiding overflow. Link: https://www.qualys.com/2024/01/30/qsort.txt [1] --- tools/rv_histogram.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/rv_histogram.c b/tools/rv_histogram.c index 4ae674901..e65fdd4ca 100644 --- a/tools/rv_histogram.c +++ b/tools/rv_histogram.c @@ -51,12 +51,26 @@ static rv_hist_t rv_reg_stats[] = { static int cmp_dec(const void *a, const void *b) { - return ((rv_hist_t *) b)->freq - ((rv_hist_t *) a)->freq; + const size_t a_freq = ((rv_hist_t *) a)->freq; + const size_t b_freq = ((rv_hist_t *) b)->freq; + + if (a_freq > b_freq) + return -1; + if (a_freq < b_freq) + return 1; + return 0; } static int cmp_asc(const void *a, const void *b) { - return ((rv_hist_t *) a)->freq - ((rv_hist_t *) b)->freq; + const size_t a_freq = ((rv_hist_t *) a)->freq; + const size_t b_freq = ((rv_hist_t *) b)->freq; + + if (a_freq < b_freq) + return -1; + if (a_freq > b_freq) + return 1; + return 0; } /* used to adjust the length of histogram bar */