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]
Date:   Mon, 23 May 2022 15:18:10 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Laurent Dufour <ldufour@...ux.ibm.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
CC:     "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v10 10/13] powerpc/mm: Move get_unmapped_area functions to
 slice.c



Le 23/05/2022 à 14:27, Laurent Dufour a écrit :
> On 09/04/2022, 19:17:34, Christophe Leroy wrote:
>> hugetlb_get_unmapped_area() is now identical to the
>> generic version if only RADIX is enabled, so move it
>> to slice.c and let it fallback on the generic one
>> when HASH MMU is not compiled in.
>>
>> Do the same with arch_get_unmapped_area() and
>> arch_get_unmapped_area_topdown().
> 
> This breaks the build when CONFIG_PPC_64S_HASH_MMU is not set.
> 
> The root cause is that arch/powerpc/mm/book3s64/slice.c is not built if
> !CONFIG_PPC_64S_HASH_MMU.
> The commit f693d38d9468 (powerpc/mm: Remove CONFIG_PPC_MM_SLICES,
> 2022-04-09) builds slice.c only if CONFIG_PPC_64S_HASH_MMU.

I think the root cause is slice.h is being included allthough 
!CONFIG_PPC_64S_HASH_MMU, which leads to HAVE_ARCH_UNMAPPED_AREA and 
HAVE_ARCH_UNMAPPED_AREA_TOPDOWN being defined while they shouldn't

Wondering why I didn't see that before.

slice.h gets included via asm/book3s/64/mmu-hash.h

I was able to build microwatt_defconfig with the following changes:

diff --git a/arch/powerpc/include/asm/book3s/64/slice.h 
b/arch/powerpc/include/asm/book3s/64/slice.h
index b8eb4ad271b9..5fbe18544cbd 100644
--- a/arch/powerpc/include/asm/book3s/64/slice.h
+++ b/arch/powerpc/include/asm/book3s/64/slice.h
@@ -4,11 +4,13 @@

  #ifndef __ASSEMBLY__

+#ifdef CONFIG_PPC_64S_HASH_MMU
  #ifdef CONFIG_HUGETLB_PAGE
  #define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
  #endif
  #define HAVE_ARCH_UNMAPPED_AREA
  #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
+#endif

  #define SLICE_LOW_SHIFT		28
  #define SLICE_LOW_TOP		(0x100000000ul)
diff --git a/arch/powerpc/sysdev/xics/ics-native.c 
b/arch/powerpc/sysdev/xics/ics-native.c
index e33b77da861e..112c8a1e8159 100644
--- a/arch/powerpc/sysdev/xics/ics-native.c
+++ b/arch/powerpc/sysdev/xics/ics-native.c
@@ -15,6 +15,7 @@
  #include <linux/init.h>
  #include <linux/cpu.h>
  #include <linux/of.h>
+#include <linux/of_address.h>
  #include <linux/spinlock.h>
  #include <linux/msi.h>
  #include <linux/list.h>


> 
> I'm wondering if these functions have to be moved in slice.c
> 
> Attached is the config file I used.
> 
>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>> ---
>>   arch/powerpc/include/asm/book3s/64/mmu.h   |  6 ----
>>   arch/powerpc/include/asm/book3s/64/slice.h |  6 ++++
>>   arch/powerpc/mm/book3s64/slice.c           | 42 ++++++++++++++++++++++
>>   arch/powerpc/mm/hugetlbpage.c              | 21 -----------
>>   arch/powerpc/mm/mmap.c                     | 36 -------------------
>>   5 files changed, 48 insertions(+), 63 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
>> index 006cbec70ffe..570a4960cf17 100644
>> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
>> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
>> @@ -4,12 +4,6 @@
>>   
>>   #include <asm/page.h>
>>   
>> -#ifdef CONFIG_HUGETLB_PAGE
>> -#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>> -#endif
>> -#define HAVE_ARCH_UNMAPPED_AREA
>> -#define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
>> -
>>   #ifndef __ASSEMBLY__
>>   /*
>>    * Page size definition
>> diff --git a/arch/powerpc/include/asm/book3s/64/slice.h b/arch/powerpc/include/asm/book3s/64/slice.h
>> index 5b0f7105bc8b..b8eb4ad271b9 100644
>> --- a/arch/powerpc/include/asm/book3s/64/slice.h
>> +++ b/arch/powerpc/include/asm/book3s/64/slice.h
>> @@ -4,6 +4,12 @@
>>   
>>   #ifndef __ASSEMBLY__
>>   
>> +#ifdef CONFIG_HUGETLB_PAGE
>> +#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>> +#endif
>> +#define HAVE_ARCH_UNMAPPED_AREA
>> +#define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
>> +
>>   #define SLICE_LOW_SHIFT		28
>>   #define SLICE_LOW_TOP		(0x100000000ul)
>>   #define SLICE_NUM_LOW		(SLICE_LOW_TOP >> SLICE_LOW_SHIFT)
>> diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
>> index e4382713746d..03681042b807 100644
>> --- a/arch/powerpc/mm/book3s64/slice.c
>> +++ b/arch/powerpc/mm/book3s64/slice.c
>> @@ -639,6 +639,32 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>   }
>>   EXPORT_SYMBOL_GPL(slice_get_unmapped_area);
>>   
>> +unsigned long arch_get_unmapped_area(struct file *filp,
>> +				     unsigned long addr,
>> +				     unsigned long len,
>> +				     unsigned long pgoff,
>> +				     unsigned long flags)
>> +{
>> +	if (radix_enabled())
>> +		return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
>> +
>> +	return slice_get_unmapped_area(addr, len, flags,
>> +				       mm_ctx_user_psize(&current->mm->context), 0);
>> +}
>> +
>> +unsigned long arch_get_unmapped_area_topdown(struct file *filp,
>> +					     const unsigned long addr0,
>> +					     const unsigned long len,
>> +					     const unsigned long pgoff,
>> +					     const unsigned long flags)
>> +{
>> +	if (radix_enabled())
>> +		return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
>> +
>> +	return slice_get_unmapped_area(addr0, len, flags,
>> +				       mm_ctx_user_psize(&current->mm->context), 1);
>> +}
>> +
>>   unsigned int notrace get_slice_psize(struct mm_struct *mm, unsigned long addr)
>>   {
>>   	unsigned char *psizes;
>> @@ -766,4 +792,20 @@ unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
>>   
>>   	return 1UL << mmu_psize_to_shift(get_slice_psize(vma->vm_mm, vma->vm_start));
>>   }
>> +
>> +static int file_to_psize(struct file *file)
>> +{
>> +	struct hstate *hstate = hstate_file(file);
>> +	return shift_to_mmu_psize(huge_page_shift(hstate));
>> +}
>> +
>> +unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
>> +					unsigned long len, unsigned long pgoff,
>> +					unsigned long flags)
>> +{
>> +	if (radix_enabled())
>> +		return generic_hugetlb_get_unmapped_area(file, addr, len, pgoff, flags);
>> +
>> +	return slice_get_unmapped_area(addr, len, flags, file_to_psize(file), 1);
>> +}
>>   #endif
>> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
>> index a87c886042e9..b282af39fcf6 100644
>> --- a/arch/powerpc/mm/hugetlbpage.c
>> +++ b/arch/powerpc/mm/hugetlbpage.c
>> @@ -542,27 +542,6 @@ struct page *follow_huge_pd(struct vm_area_struct *vma,
>>   	return page;
>>   }
>>   
>> -#ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>> -static inline int file_to_psize(struct file *file)
>> -{
>> -	struct hstate *hstate = hstate_file(file);
>> -	return shift_to_mmu_psize(huge_page_shift(hstate));
>> -}
>> -
>> -unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
>> -					unsigned long len, unsigned long pgoff,
>> -					unsigned long flags)
>> -{
>> -	if (radix_enabled())
>> -		return generic_hugetlb_get_unmapped_area(file, addr, len,
>> -						       pgoff, flags);
>> -#ifdef CONFIG_PPC_64S_HASH_MMU
>> -	return slice_get_unmapped_area(addr, len, flags, file_to_psize(file), 1);
>> -#endif
>> -	BUG();
>> -}
>> -#endif
>> -
>>   bool __init arch_hugetlb_valid_size(unsigned long size)
>>   {
>>   	int shift = __ffs(size);
>> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
>> index 46781d0103d1..5972d619d274 100644
>> --- a/arch/powerpc/mm/mmap.c
>> +++ b/arch/powerpc/mm/mmap.c
>> @@ -80,42 +80,6 @@ static inline unsigned long mmap_base(unsigned long rnd,
>>   	return PAGE_ALIGN(DEFAULT_MAP_WINDOW - gap - rnd);
>>   }
>>   
>> -#ifdef HAVE_ARCH_UNMAPPED_AREA
>> -unsigned long arch_get_unmapped_area(struct file *filp,
>> -				     unsigned long addr,
>> -				     unsigned long len,
>> -				     unsigned long pgoff,
>> -				     unsigned long flags)
>> -{
>> -	if (radix_enabled())
>> -		return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
>> -
>> -#ifdef CONFIG_PPC_64S_HASH_MMU
>> -	return slice_get_unmapped_area(addr, len, flags,
>> -				       mm_ctx_user_psize(&current->mm->context), 0);
>> -#else
>> -	BUG();
>> -#endif
>> -}
>> -
>> -unsigned long arch_get_unmapped_area_topdown(struct file *filp,
>> -					     const unsigned long addr0,
>> -					     const unsigned long len,
>> -					     const unsigned long pgoff,
>> -					     const unsigned long flags)
>> -{
>> -	if (radix_enabled())
>> -		return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
>> -
>> -#ifdef CONFIG_PPC_64S_HASH_MMU
>> -	return slice_get_unmapped_area(addr0, len, flags,
>> -				       mm_ctx_user_psize(&current->mm->context), 1);
>> -#else
>> -	BUG();
>> -#endif
>> -}
>> -#endif /* HAVE_ARCH_UNMAPPED_AREA */
>> -
>>   /*
>>    * This function, called very early during the creation of a new
>>    * process VM image, sets up which VM layout function to use:

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ