linux-kernel - Re: [PATCH v6 1/4] mm: hugetlb_vmemmap: introduce STRUCT_PAGE_SIZE_IS_POWER_OF

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: <CAMZfGtXsMZ7ZwAzV5yZ39TB-=Q7Lw8oyjq2m65SXqHGwHQfj9w@mail.gmail.com>
Date:   Thu, 31 Mar 2022 10:52:58 +0800
From:   Muchun Song <songmuchun@...edance.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Jonathan Corbet <corbet@....net>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Iurii Zaikin <yzaikin@...gle.com>,
        Oscar Salvador <osalvador@...e.de>,
        David Hildenbrand <david@...hat.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        Xiongchun duan <duanxiongchun@...edance.com>,
        Muchun Song <smuchun@...il.com>
Subject: Re: [PATCH v6 1/4] mm: hugetlb_vmemmap: introduce STRUCT_PAGE_SIZE_IS_POWER_OF_2

On Thu, Mar 31, 2022 at 10:28 AM Andrew Morton
<akpm@...ux-foundation.org> wrote:
>
> On Wed, 30 Mar 2022 23:37:42 +0800 Muchun Song <songmuchun@...edance.com> wrote:
>
> > If the size of "struct page" is not the power of two and this
> > feature is enabled,
>
> What is "this feature"?   Let's spell it out?

Will do.

>
> > 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
> > macro.  Then we could prevent anyone do any unexpected
> > configuration.
> >
> > ...
> >
> > --- /dev/null
> > +++ b/mm/struct_page_size.c
> > @@ -0,0 +1,20 @@
> > +// 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)
> > +{
> > +     if (is_power_of_2(sizeof(struct page)))
> > +             DEFINE(STRUCT_PAGE_SIZE_IS_POWER_OF_2, is_power_of_2(sizeof(struct page)));
>
> Why not
>
>         DEFINE(STRUCT_PAGE_SIZE_IS_POWER_OF_2, 1);
>

Yep, this is more simple.  But the 2nd parameter of DEFINE() will
go into the comments.  I want to make it more clear when someone
reads the code of this macro.  The two different sentences will
generate the following two different comments.  Which one do
you think is better?

#define STRUCT_PAGE_SIZE_IS_POWER_OF_2 1 /*
is_power_of_2(sizeof(struct page)) */
#define STRUCT_PAGE_SIZE_IS_POWER_OF_2 1 /* 1 */

Thanks.