| 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
| ||
|
Message-ID: <c3071359-c5d3-4247-7f16-6f61b2fa0756@c-s.fr>
Date: Thu, 19 Mar 2020 08:00:59 +0100
From: Christophe Leroy <christophe.leroy@....fr>
To: Mike Kravetz <mike.kravetz@...cle.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linuxppc-dev@...ts.ozlabs.org, linux-riscv@...ts.infradead.org,
linux-s390@...r.kernel.org, sparclinux@...r.kernel.org,
linux-doc@...r.kernel.org
Cc: Albert Ou <aou@...s.berkeley.edu>,
Andrew Morton <akpm@...ux-foundation.org>,
Vasily Gorbik <gor@...ux.ibm.com>,
Jonathan Corbet <corbet@....net>,
Catalin Marinas <catalin.marinas@....com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Heiko Carstens <heiko.carstens@...ibm.com>,
Christian Borntraeger <borntraeger@...ibm.com>,
Ingo Molnar <mingo@...hat.com>,
Palmer Dabbelt <palmer@...belt.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Paul Mackerras <paulus@...ba.org>,
Thomas Gleixner <tglx@...utronix.de>,
Longpeng <longpeng2@...wei.com>, Will Deacon <will@...nel.org>,
"David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
Le 18/03/2020 à 23:06, Mike Kravetz a écrit :
> The architecture independent routine hugetlb_default_setup sets up
> the default huge pages size. It has no way to verify if the passed
> value is valid, so it accepts it and attempts to validate at a later
> time. This requires undocumented cooperation between the arch specific
> and arch independent code.
>
> For architectures that support more than one huge page size, provide
> a routine arch_hugetlb_valid_size to validate a huge page size.
> hugetlb_default_setup can use this to validate passed values.
>
> arch_hugetlb_valid_size will also be used in a subsequent patch to
> move processing of the "hugepagesz=" in arch specific code to a common
> routine in arch independent code.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@...cle.com>
> ---
> arch/arm64/include/asm/hugetlb.h | 2 ++
> arch/arm64/mm/hugetlbpage.c | 19 ++++++++++++++-----
> arch/powerpc/include/asm/hugetlb.h | 3 +++
> arch/powerpc/mm/hugetlbpage.c | 20 +++++++++++++-------
> arch/riscv/include/asm/hugetlb.h | 3 +++
> arch/riscv/mm/hugetlbpage.c | 28 ++++++++++++++++++----------
> arch/s390/include/asm/hugetlb.h | 3 +++
> arch/s390/mm/hugetlbpage.c | 18 +++++++++++++-----
> arch/sparc/include/asm/hugetlb.h | 3 +++
> arch/sparc/mm/init_64.c | 23 ++++++++++++++++-------
> arch/x86/include/asm/hugetlb.h | 3 +++
> arch/x86/mm/hugetlbpage.c | 21 +++++++++++++++------
> include/linux/hugetlb.h | 7 +++++++
> mm/hugetlb.c | 16 +++++++++++++---
> 14 files changed, 126 insertions(+), 43 deletions(-)
>
[snip]
> diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
> index bd6504c28c2f..3b5939016955 100644
> --- a/arch/powerpc/include/asm/hugetlb.h
> +++ b/arch/powerpc/include/asm/hugetlb.h
> @@ -64,6 +64,9 @@ static inline void arch_clear_hugepage_flags(struct page *page)
> {
> }
>
> +#define arch_hugetlb_valid_size arch_hugetlb_valid_size
> +extern bool __init arch_hugetlb_valid_size(unsigned long long size);
Don't add 'extern' keyword, it is irrelevant for a function declaration.
checkpatch --strict doesn't like it either
(https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/12318//artifact/linux/checkpatch.log)
> +
> #include <asm-generic/hugetlb.h>
>
> #else /* ! CONFIG_HUGETLB_PAGE */
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 33b3461d91e8..b78f660252f3 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -558,7 +558,7 @@ unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
> return vma_kernel_pagesize(vma);
> }
>
> -static int __init add_huge_page_size(unsigned long long size)
> +bool __init arch_hugetlb_valid_size(unsigned long long size)
> {
> int shift = __ffs(size);
> int mmu_psize;
> @@ -566,20 +566,26 @@ static int __init add_huge_page_size(unsigned long long size)
> /* Check that it is a page size supported by the hardware and
> * that it fits within pagetable and slice limits. */
> if (size <= PAGE_SIZE || !is_power_of_2(size))
> - return -EINVAL;
> + return false;
>
> mmu_psize = check_and_get_huge_psize(shift);
> if (mmu_psize < 0)
> - return -EINVAL;
> + return false;
>
> BUG_ON(mmu_psize_defs[mmu_psize].shift != shift);
>
> - /* Return if huge page size has already been setup */
> - if (size_to_hstate(size))
> - return 0;
> + return true;
> +}
>
> - hugetlb_add_hstate(shift - PAGE_SHIFT);
> +static int __init add_huge_page_size(unsigned long long size)
> +{
> + int shift = __ffs(size);
> +
> + if (!arch_hugetlb_valid_size(size))
> + return -EINVAL;
>
> + if (!size_to_hstate(size))
> + hugetlb_add_hstate(shift - PAGE_SHIFT);
> return 0;
> }
>
[snip]
> diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
> index 5bfd5aef5378..51e6208fdeec 100644
> --- a/arch/x86/mm/hugetlbpage.c
> +++ b/arch/x86/mm/hugetlbpage.c
> @@ -181,16 +181,25 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> #endif /* CONFIG_HUGETLB_PAGE */
>
> #ifdef CONFIG_X86_64
> +bool __init arch_hugetlb_valid_size(unsigned long long size)
> +{
> + if (size == PMD_SIZE)
> + return true;
> + else if (size == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES))
> + return true;
> + else
> + return false;
> +}
> +
> static __init int setup_hugepagesz(char *opt)
> {
> - unsigned long ps = memparse(opt, &opt);
> - if (ps == PMD_SIZE) {
> - hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
> - } else if (ps == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES)) {
> - hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> + unsigned long long ps = memparse(opt, &opt);
> +
> + if (arch_hugetlb_valid_size(ps)) {
> + hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
> } else {
> hugetlb_bad_size();
> - printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n",
> + printk(KERN_ERR "hugepagesz: Unsupported page size %llu M\n",
> ps >> 20);
Nowadays we use pr_err() instead of printk.
It would also likely allow you to have everything fit on a single line.
> return 0;
> }
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index b831e9fa1a26..33343eb980d0 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -678,6 +678,13 @@ static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
> return &mm->page_table_lock;
> }
>
> +#ifndef arch_hugetlb_valid_size
> +static inline bool arch_hugetlb_valid_size(unsigned long long size)
> +{
> + return (size == HPAGE_SIZE);
Not sure the ( ) are necessary.
> +}
> +#endif
> +
> #ifndef hugepages_supported
> /*
> * Some platform decide whether they support huge pages at boot
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index d8ebd876871d..2f99359b93af 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3224,12 +3224,22 @@ static int __init hugetlb_nrpages_setup(char *s)
> }
> __setup("hugepages=", hugetlb_nrpages_setup);
>
> -static int __init hugetlb_default_setup(char *s)
> +static int __init default_hugepagesz_setup(char *s)
> {
> - default_hstate_size = memparse(s, &s);
> + unsigned long long size;
Why unsigned long long ?
default_hstate_size is long.
I can't imagine 32 bits platforms having a hugepage with a 64 bits size.
> + char *saved_s = s;
> +
> + size = memparse(s, &s);
The updated s is not reused after that so you can pass NULL instead of
&s and then you don't need the saved_s.
> +
> + if (!arch_hugetlb_valid_size(size)) {
> + pr_err("HugeTLB: unsupported default_hugepagesz %s\n", saved_s);
> + return 0;
> + }
> +
> + default_hstate_size = size;
> return 1;
> }
> -__setup("default_hugepagesz=", hugetlb_default_setup);
> +__setup("default_hugepagesz=", default_hugepagesz_setup);
>
> static unsigned int cpuset_mems_nr(unsigned int *array)
> {
>
Christophe
Powered by blists - more mailing lists