[PATCH] perf bench: Fix undefined behavior in cmpworker()

[PATCH] perf bench: Fix undefined behavior in cmpworker()

[Kuan-Wei Chiu]

The comparison function cmpworker() does not comply with the C
standard's requirements for qsort() comparison functions. Specifically,
it returns 0 when w1->tid < w2->tid, which is incorrect. According to
the standard, the function must return a negative value in such cases
to preserve proper ordering.

This violation causes undefined behavior, potentially leading to issues
such as memory corruption in certain versions of glibc [1].

Fix the issue by returning -1 when w1->tid < w2->tid, ensuring
compliance with the C standard and preventing undefined behavior.

Link: https://www.qualys.com/2024/01/30/qsort.txt [1]
Fixes: 121dd9ea0116 ("perf bench: Add epoll parallel epoll_wait benchmark")
Cc: stable@vger.kernel.org
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 tools/perf/bench/epoll-wait.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/bench/epoll-wait.c b/tools/perf/bench/epoll-wait.c
index ef5c4257844d..4868d610e9bf 100644
--- a/tools/perf/bench/epoll-wait.c
+++ b/tools/perf/bench/epoll-wait.c
@@ -420,7 +420,7 @@ static int cmpworker(const void *p1, const void *p2)
 
 	struct worker *w1 = (struct worker *) p1;
 	struct worker *w2 = (struct worker *) p2;
-	return w1->tid > w2->tid;
+	return w1->tid > w2->tid ? 1 : -1;
 }
 
 int bench_epoll_wait(int argc, const char **argv)
-- 
2.34.1

Re: [PATCH] perf bench: Fix undefined behavior in cmpworker()

[Kuan-Wei Chiu]

On Mon, Dec 09, 2024 at 10:57:28PM +0800, Kuan-Wei Chiu wrote:
> The comparison function cmpworker() does not comply with the C
> standard's requirements for qsort() comparison functions. Specifically,
> it returns 0 when w1->tid < w2->tid, which is incorrect. According to
> the standard, the function must return a negative value in such cases
> to preserve proper ordering.
> 
> This violation causes undefined behavior, potentially leading to issues
> such as memory corruption in certain versions of glibc [1].
> 
> Fix the issue by returning -1 when w1->tid < w2->tid, ensuring
> compliance with the C standard and preventing undefined behavior.
>
I reviewed my commit message again and thought it might be clearer to
explicitly mention, as in the previous patch, that the issue stems from
violating symmetry and transitivity. The current cmpworker() can result
in x > y but y = x, leading to undefined behavior. I'll wait for review
comments before updating the patch description.

Regards,
Kuan-Wei