linux-kernel - Re: [PATCH v1 01/16] mm: hugetlb: Add huge page size param to huge_ptep_get_and

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: <ed3f2549-cacc-4eaa-80c3-3f220835c9e6@arm.com>
Date: Thu, 6 Feb 2025 12:15:47 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Anshuman Khandual <anshuman.khandual@....com>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Muchun Song <muchun.song@...ux.dev>,
 Pasha Tatashin <pasha.tatashin@...een.com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Uladzislau Rezki <urezki@...il.com>, Christoph Hellwig <hch@...radead.org>,
 Mark Rutland <mark.rutland@....com>, Ard Biesheuvel <ardb@...nel.org>,
 Dev Jain <dev.jain@....com>, Alexandre Ghiti <alexghiti@...osinc.com>,
 Steve Capper <steve.capper@...aro.org>, Kevin Brodsky <kevin.brodsky@....com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, Huacai Chen <chenhuacai@...nel.org>,
 Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
 "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
 Helge Deller <deller@....de>, Madhavan Srinivasan <maddy@...ux.ibm.com>,
 Michael Ellerman <mpe@...erman.id.au>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
 Alexander Gordeev <agordeev@...ux.ibm.com>,
 Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
 "David S. Miller" <davem@...emloft.net>,
 Andreas Larsson <andreas@...sler.com>, stable@...r.kernel.org
Subject: Re: [PATCH v1 01/16] mm: hugetlb: Add huge page size param to
 huge_ptep_get_and_clear()

Thanks for the review!

On 06/02/2025 05:03, Anshuman Khandual wrote:
> 
> 
> On 2/5/25 20:39, Ryan Roberts wrote:
>> In order to fix a bug, arm64 needs to be told the size of the huge page
>> for which the huge_pte is being set in huge_ptep_get_and_clear().
>> Provide for this by adding an `unsigned long sz` parameter to the
>> function. This follows the same pattern as huge_pte_clear() and
>> set_huge_pte_at().
>>
>> This commit makes the required interface modifications to the core mm as
>> well as all arches that implement this function (arm64, loongarch, mips,
>> parisc, powerpc, riscv, s390, sparc). The actual arm64 bug will be fixed
>> in a separate commit.
>>
>> Cc: <stable@...r.kernel.org>
>> Fixes: 66b3923a1a0f ("arm64: hugetlb: add support for PTE contiguous bit")
>> Signed-off-by: Ryan Roberts <ryan.roberts@....com>
>> ---
>>  arch/arm64/include/asm/hugetlb.h     |  4 ++--
>>  arch/arm64/mm/hugetlbpage.c          |  8 +++++---
>>  arch/loongarch/include/asm/hugetlb.h |  6 ++++--
>>  arch/mips/include/asm/hugetlb.h      |  6 ++++--
>>  arch/parisc/include/asm/hugetlb.h    |  2 +-
>>  arch/parisc/mm/hugetlbpage.c         |  2 +-
>>  arch/powerpc/include/asm/hugetlb.h   |  6 ++++--
>>  arch/riscv/include/asm/hugetlb.h     |  3 ++-
>>  arch/riscv/mm/hugetlbpage.c          |  2 +-
>>  arch/s390/include/asm/hugetlb.h      | 12 ++++++++----
>>  arch/s390/mm/hugetlbpage.c           | 10 ++++++++--
>>  arch/sparc/include/asm/hugetlb.h     |  2 +-
>>  arch/sparc/mm/hugetlbpage.c          |  2 +-
>>  include/asm-generic/hugetlb.h        |  2 +-
>>  include/linux/hugetlb.h              |  4 +++-
>>  mm/hugetlb.c                         |  4 ++--
>>  16 files changed, 48 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
>> index c6dff3e69539..03db9cb21ace 100644
>> --- a/arch/arm64/include/asm/hugetlb.h
>> +++ b/arch/arm64/include/asm/hugetlb.h
>> @@ -42,8 +42,8 @@ extern int huge_ptep_set_access_flags(struct vm_area_struct *vma,
>>  				      unsigned long addr, pte_t *ptep,
>>  				      pte_t pte, int dirty);
>>  #define __HAVE_ARCH_HUGE_PTEP_GET_AND_CLEAR
>> -extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
>> -				     unsigned long addr, pte_t *ptep);
>> +extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
>> +				     pte_t *ptep, unsigned long sz);
> 
> If VMA could be passed instead of MM, the size of the huge page can
> be derived via huge_page_size(hstate_vma(vma)) and another argument
> here need not be added. Also MM can be derived from VMA if required.

I considered this approach; infact that's what I first implemented when fixing
an equivalent bug on set_huge_pte_at() in the past. But that was problematic
because there are some cases where the helper is used for kernel mappings (see
vmalloc) and there is no VMA to pass in that case. See [1].

To fix this bug, usage of this helper for kernel mappings is not an issue (yet)
so I guess technically it could be fixed by passing VMA. But later in this
series I start using huge_ptep_get_and_clear() for the vmap (see patch 11) so it
would break at that point.

Another approach I considered was to allocate a spare swap-pte bit (we have a
few) to indicate PTE_CONT for non-present PTEs. Then no API change is required
at all. But given set_huge_pte_at() and huge_pte_clear() already pass "sz", it
seemed best just to keep things simple and follow that pattern.

[1] https://lore.kernel.org/all/20230922115804.2043771-1-ryan.roberts@arm.com/

Thanks,
Ryan