linux-kernel - Re: [PATCH v2 1/2] mm/slub: Fix cmp_loc_by_count() to return 0 when counts are equal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aLEMtCwuRzfN3wPc@hyeyoo>
Date: Fri, 29 Aug 2025 11:13:08 +0900
From: Harry Yoo <harry.yoo@...cle.com>
To: Kuan-Wei Chiu <visitorckw@...il.com>
Cc: vbabka@...e.cz, akpm@...ux-foundation.org, cl@...two.org,
        rientjes@...gle.com, roman.gushchin@...ux.dev, glittao@...il.com,
        jserv@...s.ncku.edu.tw, chuang@...nycu.edu.tw, cfmc.cs13@...u.edu.tw,
        jhcheng.cs13@...u.edu.tw, c.yuanhaur@...tl.edu, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Joshua Hahn <joshua.hahnjy@...il.com>
Subject: Re: [PATCH v2 1/2] mm/slub: Fix cmp_loc_by_count() to return 0 when
 counts are equal

On Tue, Aug 26, 2025 at 02:23:14PM +0800, Kuan-Wei Chiu wrote:
> The comparison function cmp_loc_by_count() used for sorting stack trace
> locations in debugfs currently returns -1 if a->count > b->count and 1
> otherwise. This breaks the antisymmetry property required by sort(),
> because when two counts are equal, both cmp(a, b) and cmp(b, a) return
> 1.
> 
> This can lead to undefined or incorrect ordering results. Fix it by
> updating the comparison logic to explicitly handle the case when counts
> are equal, and use cmp_int() to ensure the comparison function adheres
> to the required mathematical properties of antisymmetry.
> 
> Fixes: 553c0369b3e1 ("mm/slub: sort debugfs output by frequency of stack traces")
> Reviewed-by: Joshua Hahn <joshua.hahnjy@...il.com>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@...il.com>
> ---

While the author withdrew the claim that it definitely leads to incorrect
results, it remains true that the API requires both transitivity and
antisymmetry for correctness, so:

Reviewed-by: Harry Yoo <harry.yoo@...cle.com>

-- 
Cheers,
Harry / Hyeonggon

>  mm/slub.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 30003763d224..081816ff89ab 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -7716,10 +7716,7 @@ static int cmp_loc_by_count(const void *a, const void *b, const void *data)
>  	struct location *loc1 = (struct location *)a;
>  	struct location *loc2 = (struct location *)b;
>  
> -	if (loc1->count > loc2->count)
> -		return -1;
> -	else
> -		return 1;
> +	return cmp_int(loc2->count, loc1->count);
>  }
>  
>  static void *slab_debugfs_start(struct seq_file *seq, loff_t *ppos)
> -- 
> 2.34.1