linux-kernel - Re: [PATCH] Fix the data type inconsistency issue of min (tier, MAX_CR_TIERS-1) in read_ctrl

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] [day] [month] [year] [list]

Message-Id: <20250811214053.857168fd35e70e73dee1583d@linux-foundation.org>
Date: Mon, 11 Aug 2025 21:40:53 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: David Hildenbrand <david@...hat.com>
Cc: Qingshuang Fu <fffsqian@....com>, hannes@...xchg.org, mhocko@...nel.org,
 zhengqi.arch@...edance.com, lorenzo.stoakes@...cle.com, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, Qingshuang Fu <fuqingshuang@...inos.cn>,
 Yuanchu Xie <yuanchu@...gle.com>, Axel Rasmussen <axelrasmussen@...gle.com>
Subject: Re: [PATCH] Fix the data type inconsistency issue of min (tier,
 MAX_CR_TIERS-1) in read_ctrl_pos

On Fri, 8 Aug 2025 09:35:19 +0200 David Hildenbrand <david@...hat.com> wrote:

> > 
> > Due to the fact that the tier data type in min (tier, MAX_CR_TIERS -1)
> > is int,but MAX_CR_TIERS is an unsigned type, directly using
> > the min function for comparison will result in an error:
> > from mm/vmscan.c:15:
> > mm/vmscan.c: In function ‘read_ctrl_pos’:
> > ./include/linux/build_bug.h:78:41: error: static assertion failed:
> > "min(tier, 4U - 1) signedness error, fix types or
> > consider umin() before min_t()"
> > And MAX_CR_TIERS is a macro definition defined as 4U,
> > so min_t can be used to convert it to int type before
> > performing the minimum value operation.
> > 
> 
> Please use empty lines to make the description easier to read. Also, I 
> think you can simplify this heavily.
> 
> We should add
> 
> Fixes: 37a260870f2c ("mm/mglru: rework type selection")

I'm not liking read_ctrl_pos() much.

Local variable `i' has the rottenest possible name.  In this case it is
a "tier", so let's call it that.  And its type should be unsigned.

But an incoming arg has that name.  Probably inappropriately.  I'm
suspecting that something like base_tier or start_tier would be more
descriptive.  Hard to tell, because read_ctrl_pos() forgot to get
documented.  And this arg should have unsigned type also.

(cc mglru maintainers, who have yet to reveal themselves in MAINTAINERS)