| 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: <CAK7LNAQoRfo_L0xa+rCBNWcYYSjV-HSTOQcQ-q0BUs9daKH94g@mail.gmail.com>
Date: Fri, 25 Mar 2022 14:09:56 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Muchun Song <songmuchun@...edance.com>
Cc: Jonathan Corbet <corbet@....net>, mike.kravetz@...cle.com,
Andrew Morton <akpm@...ux-foundation.org>,
"Luis R. Rodriguez" <mcgrof@...nel.org>,
Kees Cook <keescook@...omium.org>,
Iurii Zaikin <yzaikin@...gle.com>,
Oscar Salvador <osalvador@...e.de>,
David Hildenbrand <david@...hat.com>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux Memory Management List <linux-mm@...ck.org>,
duanxiongchun@...edance.com, smuchun@...il.com
Subject: Re: [PATCH v5 1/4] mm: hugetlb_vmemmap: introduce STRUCT_PAGE_SIZE_IS_POWER_OF_2
On Wed, Mar 23, 2022 at 9:57 PM Muchun Song <songmuchun@...edance.com> wrote:
>
> If the size of "struct page" is not the power of two and this
> feature is enabled, then the vmemmap pages of HugeTLB will be
> corrupted after remapping (panic is about to happen in theory).
> But this only exists when !CONFIG_MEMCG && !CONFIG_SLUB on
> x86_64. However, it is not a conventional configuration nowadays.
> So it is not a real word issue, just the result of a code review.
> But we have to prevent anyone from configuring that combined
> configuration. In order to avoid many checks like "is_power_of_2
> (sizeof(struct page))" through mm/hugetlb_vmemmap.c. Introduce
> STRUCT_PAGE_SIZE_IS_POWER_OF_2 to detect if the size of struct
> page is power of 2 and make this feature depends on this new
> config. Then we could prevent anyone do any unexpected
> configuration.
>
> Signed-off-by: Muchun Song <songmuchun@...edance.com>
> Suggested-by: Luis Chamberlain <mcgrof@...nel.org>
> ---
> Kbuild | 14 ++++++++++++++
> fs/Kconfig | 1 +
> include/linux/mm_types.h | 2 ++
> mm/Kconfig | 3 +++
> mm/hugetlb_vmemmap.c | 6 ------
> mm/struct_page_size.c | 19 +++++++++++++++++++
> scripts/check_struct_page_po2.sh | 9 +++++++++
> 7 files changed, 48 insertions(+), 6 deletions(-)
> create mode 100644 mm/struct_page_size.c
> create mode 100755 scripts/check_struct_page_po2.sh
>
> diff --git a/Kbuild b/Kbuild
> index fa441b98c9f6..21415c3b2728 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -37,6 +37,20 @@ $(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
> $(call filechk,offsets,__ASM_OFFSETS_H__)
>
> #####
> +# Generate struct_page_size.h.
> +
> +struct_page_size-file := include/generated/struct_page_size.h
> +
> +always-y := $(struct_page_size-file)
> +targets := mm/struct_page_size.s
> +
> +mm/struct_page_size.s: $(timeconst-file) $(bounds-file)
> +
> +$(struct_page_size-file): mm/struct_page_size.s FORCE
> + $(call filechk,offsets,__LINUX_STRUCT_PAGE_SIZE_H__)
> + $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
No, please do not do this.
It is terrible to feed back this to Kconfig again.
If you know this happens on !CONFIG_MEMCG && !CONFIG_SLUB on x86_64,
why don't you add this dependency directly?
If you want to avoid the run-time check,
why don't you use BUILD_BUG_ON() ?
> +
> +#####
> # Check for missing system calls
>
> always-y += missing-syscalls
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 7f2455e8e18a..856d2e9f5aef 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -249,6 +249,7 @@ config HUGETLB_PAGE_FREE_VMEMMAP
> def_bool HUGETLB_PAGE
> depends on X86_64
> depends on SPARSEMEM_VMEMMAP
> + depends on STRUCT_PAGE_SIZE_IS_POWER_OF_2
>
> config HUGETLB_PAGE_FREE_VMEMMAP_DEFAULT_ON
> bool "Default freeing vmemmap pages of HugeTLB to on"
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 8834e38c06a4..5fbff44a4310 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -223,6 +223,7 @@ struct page {
> #endif
> } _struct_page_alignment;
>
> +#ifndef __GENERATING_STRUCT_PAGE_SIZE_IS_POWER_OF_2_H
> /**
> * struct folio - Represents a contiguous set of bytes.
> * @flags: Identical to the page flags.
> @@ -844,5 +845,6 @@ enum fault_flag {
> FAULT_FLAG_INSTRUCTION = 1 << 8,
> FAULT_FLAG_INTERRUPTIBLE = 1 << 9,
> };
> +#endif /* !__GENERATING_STRUCT_PAGE_SIZE_IS_POWER_OF_2_H */
>
> #endif /* _LINUX_MM_TYPES_H */
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 034d87953600..9314bd34f49e 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -2,6 +2,9 @@
>
> menu "Memory Management options"
>
> +config STRUCT_PAGE_SIZE_IS_POWER_OF_2
> + def_bool $(success,test "$(shell, $(srctree)/scripts/check_struct_page_po2.sh)" = 1)
> +
> config SELECT_MEMORY_MODEL
> def_bool y
> depends on ARCH_SELECT_MEMORY_MODEL
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 791626983c2e..33ecb77c2b2a 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -194,12 +194,6 @@ EXPORT_SYMBOL(hugetlb_free_vmemmap_enabled_key);
>
> static int __init early_hugetlb_free_vmemmap_param(char *buf)
> {
> - /* We cannot optimize if a "struct page" crosses page boundaries. */
> - if (!is_power_of_2(sizeof(struct page))) {
> - pr_warn("cannot free vmemmap pages because \"struct page\" crosses page boundaries\n");
> - return 0;
> - }
> -
> if (!buf)
> return -EINVAL;
>
> diff --git a/mm/struct_page_size.c b/mm/struct_page_size.c
> new file mode 100644
> index 000000000000..5749609aa1b3
> --- /dev/null
> +++ b/mm/struct_page_size.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Generate definitions needed by the preprocessor.
> + * This code generates raw asm output which is post-processed
> + * to extract and format the required data.
> + */
> +
> +#define __GENERATING_STRUCT_PAGE_SIZE_IS_POWER_OF_2_H
> +/* Include headers that define the enum constants of interest */
> +#include <linux/mm_types.h>
> +#include <linux/kbuild.h>
> +#include <linux/log2.h>
> +
> +int main(void)
> +{
> + DEFINE(STRUCT_PAGE_SIZE_IS_POWER_OF_2, is_power_of_2(sizeof(struct page)));
> +
> + return 0;
> +}
> diff --git a/scripts/check_struct_page_po2.sh b/scripts/check_struct_page_po2.sh
> new file mode 100755
> index 000000000000..1764ef9a4f1d
> --- /dev/null
> +++ b/scripts/check_struct_page_po2.sh
> @@ -0,0 +1,9 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Check if the size of "struct page" is power of 2
> +
> +file="include/generated/struct_page_size.h"
> +if [ -f "$file" ]; then
> + grep STRUCT_PAGE_SIZE_IS_POWER_OF_2 "$file" | cut -d' ' -f3
> +fi
> --
> 2.11.0
>
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists