linux-kernel - Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting

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: <721894de-1b36-419b-a3d4-0ab43d607d3f@lucifer.local>
Date: Mon, 24 Nov 2025 15:21:43 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Zi Yan <ziy@...dia.com>
Cc: David Hildenbrand <david@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Nico Pache <npache@...hat.com>, Ryan Roberts <ryan.roberts@....com>,
        Dev Jain <dev.jain@....com>, Barry Song <baohua@...nel.org>,
        Lance Yang <lance.yang@...ux.dev>, Miaohe Lin <linmiaohe@...wei.com>,
        Naoya Horiguchi <nao.horiguchi@...il.com>,
        Wei Yang <richard.weiyang@...il.com>,
        Balbir Singh <balbirs@...dia.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting

On Fri, Nov 21, 2025 at 09:55:29PM -0500, Zi Yan wrote:
> The "return <error code>" statements for error checks at the beginning of
> __folio_split() skip necessary count_vm_event() and count_mthp_stat() at
> the end of the function. Fix these by replacing them with
> "ret = <error code>; goto out;".

I guess the xas_destroy() there will be a no-op in these cases!

Good spot, as David said, maybe one for stable then... not sure if necessary for
statistical stuff though?

But at the same time, maybe users will be misled if these are incorrect?

Has this bug been around since the beginning? Be curious to know if that's the
case or if it was introduced somewhere along the line?

>
> Signed-off-by: Zi Yan <ziy@...dia.com>

LGTM, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>


> ---
>  mm/huge_memory.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index ebc3ba0907fd..a42c4f29ce4f 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3954,16 +3954,20 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
>
> -	if (folio != page_folio(split_at) || folio != page_folio(lock_at))
> -		return -EINVAL;
> +	if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
>
> -	if (new_order >= old_order)
> -		return -EINVAL;
> +	if (new_order >= old_order) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
>
>  	ret = folio_check_splittable(folio, new_order, split_type,
>  				     /* warn = */ true);
>  	if (ret)
> -		return ret;
> +		goto out;
>
>  	if (is_anon) {
>  		/*
> --
> 2.51.0
>

Cheers, Lorenzo