The comparison function cmp_profile_data() violates the C standard's
requirements for qsort() comparison functions, which mandate symmetry
and transitivity:

* Symmetry: If x < y, then y > x.
* Transitivity: If x < y and y < z, then x < z.

When v1 and v2 are equal, the function incorrectly returns 1, breaking
symmetry and transitivity. This causes undefined behavior, which can
lead to memory corruption in certain versions of glibc [1].

Fix the issue by returning 0 when v1 and v2 are equal, ensuring
compliance with the C standard and preventing undefined behavior.

Link: https://www.qualys.com/2024/01/30/qsort.txt [1]
Fixes: 0f223813edd0 ("perf ftrace: Add 'profile' command")
Fixes: 74ae366c37b7 ("perf ftrace profile: Add -s/--sort option")
Cc: stable@vger.kernel.org
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 tools/perf/builtin-ftrace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 272d3c70810e..a56cf8b0a7d4 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -1151,8 +1151,9 @@ static int cmp_profile_data(const void *a, const void *b)
 
 	if (v1 > v2)
 		return -1;
-	else
+	if (v1 < v2)
 		return 1;
+	return 0;
 }
 
 static void print_profile_result(struct perf_ftrace *ftrace)
-- 
2.34.1

On Mon, 09 Dec 2024 21:42:26 +0800, Kuan-Wei Chiu wrote:

> The comparison function cmp_profile_data() violates the C standard's
> requirements for qsort() comparison functions, which mandate symmetry
> and transitivity:
> 
> * Symmetry: If x < y, then y > x.
> * Transitivity: If x < y and y < z, then x < z.
> 
> [...]

Applied to perf-tools, thanks!

Best regards,
Namhyung

Hello,

On Mon, Dec 9, 2024 at 5:42 AM Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
>
> The comparison function cmp_profile_data() violates the C standard's
> requirements for qsort() comparison functions, which mandate symmetry
> and transitivity:
>
> * Symmetry: If x < y, then y > x.
> * Transitivity: If x < y and y < z, then x < z.
>
> When v1 and v2 are equal, the function incorrectly returns 1, breaking
> symmetry and transitivity. This causes undefined behavior, which can
> lead to memory corruption in certain versions of glibc [1].
>
> Fix the issue by returning 0 when v1 and v2 are equal, ensuring
> compliance with the C standard and preventing undefined behavior.
>
> Link: https://www.qualys.com/2024/01/30/qsort.txt [1]
> Fixes: 0f223813edd0 ("perf ftrace: Add 'profile' command")
> Fixes: 74ae366c37b7 ("perf ftrace profile: Add -s/--sort option")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks for the fix.
Namhyung

> ---
>  tools/perf/builtin-ftrace.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 272d3c70810e..a56cf8b0a7d4 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -1151,8 +1151,9 @@ static int cmp_profile_data(const void *a, const void *b)
>
>         if (v1 > v2)
>                 return -1;
> -       else
> +       if (v1 < v2)
>                 return 1;
> +       return 0;
>  }
>
>  static void print_profile_result(struct perf_ftrace *ftrace)
> --
> 2.34.1
>

On Mon, Dec 09, 2024 at 09:02:24AM -0800, Namhyung Kim wrote:
> Hello,
> 
> On Mon, Dec 9, 2024 at 5:42 AM Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> >
> > The comparison function cmp_profile_data() violates the C standard's
> > requirements for qsort() comparison functions, which mandate symmetry
> > and transitivity:
> >
> > * Symmetry: If x < y, then y > x.
> > * Transitivity: If x < y and y < z, then x < z.
> >
> > When v1 and v2 are equal, the function incorrectly returns 1, breaking
> > symmetry and transitivity. This causes undefined behavior, which can
> > lead to memory corruption in certain versions of glibc [1].
> >
> > Fix the issue by returning 0 when v1 and v2 are equal, ensuring
> > compliance with the C standard and preventing undefined behavior.
> >
> > Link: https://www.qualys.com/2024/01/30/qsort.txt [1]
> > Fixes: 0f223813edd0 ("perf ftrace: Add 'profile' command")
> > Fixes: 74ae366c37b7 ("perf ftrace profile: Add -s/--sort option")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> 
> Reviewed-by: Namhyung Kim <namhyung@kernel.org>

I'm assuming you'll pick this for perf-tools, ok?

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo

On Mon, Dec 9, 2024 at 12:26 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Mon, Dec 09, 2024 at 09:02:24AM -0800, Namhyung Kim wrote:
> > Hello,
> >
> > On Mon, Dec 9, 2024 at 5:42 AM Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> > >
> > > The comparison function cmp_profile_data() violates the C standard's
> > > requirements for qsort() comparison functions, which mandate symmetry
> > > and transitivity:
> > >
> > > * Symmetry: If x < y, then y > x.
> > > * Transitivity: If x < y and y < z, then x < z.
> > >
> > > When v1 and v2 are equal, the function incorrectly returns 1, breaking
> > > symmetry and transitivity. This causes undefined behavior, which can
> > > lead to memory corruption in certain versions of glibc [1].
> > >
> > > Fix the issue by returning 0 when v1 and v2 are equal, ensuring
> > > compliance with the C standard and preventing undefined behavior.
> > >
> > > Link: https://www.qualys.com/2024/01/30/qsort.txt [1]
> > > Fixes: 0f223813edd0 ("perf ftrace: Add 'profile' command")
> > > Fixes: 74ae366c37b7 ("perf ftrace profile: Add -s/--sort option")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> >
> > Reviewed-by: Namhyung Kim <namhyung@kernel.org>
>
> I'm assuming you'll pick this for perf-tools, ok?
>
> Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Yep, sure.

Thanks,
Namhyung