linux-kernel - Re: [PATCH 08/31] kasan, page_alloc: refactor init checks in post_alloc

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+fCnZfto82vg3vGkZGNxJKOOqsOp_bpmHEd0Z350PfPJ7Y=1w@mail.gmail.com>
Date:   Mon, 6 Dec 2021 22:09:38 +0100
From:   Andrey Konovalov <andreyknvl@...il.com>
To:     Alexander Potapenko <glider@...gle.com>
Cc:     andrey.konovalov@...ux.dev, Marco Elver <elver@...gle.com>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Peter Collingbourne <pcc@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        kasan-dev <kasan-dev@...glegroups.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        Will Deacon <will@...nel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Evgenii Stepanov <eugenis@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH 08/31] kasan, page_alloc: refactor init checks in post_alloc_hook

On Thu, Dec 2, 2021 at 5:14 PM Alexander Potapenko <glider@...gle.com> wrote:
>
> On Tue, Nov 30, 2021 at 10:41 PM <andrey.konovalov@...ux.dev> wrote:
> >
> > From: Andrey Konovalov <andreyknvl@...gle.com>
> >
> > This patch separates code for zeroing memory from the code clearing tags
> > in post_alloc_hook().
> >
> > This patch is not useful by itself but makes the simplifications in
> > the following patches easier to follow.
> >
> > This patch does no functional changes.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@...gle.com>
> > ---
> >  mm/page_alloc.c | 18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 2ada09a58e4b..0561cdafce36 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -2406,19 +2406,21 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
> >                 kasan_alloc_pages(page, order, gfp_flags);
> >         } else {
> >                 bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
> > +               bool init_tags = init && (gfp_flags & __GFP_ZEROTAGS);
> >
> >                 kasan_unpoison_pages(page, order, init);
> >
> > -               if (init) {
> > -                       if (gfp_flags & __GFP_ZEROTAGS) {
> > -                               int i;
> > +               if (init_tags) {
> > +                       int i;
> >
> > -                               for (i = 0; i < 1 << order; i++)
> > -                                       tag_clear_highpage(page + i);
> > -                       } else {
> > -                               kernel_init_free_pages(page, 1 << order);
> > -                       }
> > +                       for (i = 0; i < 1 << order; i++)
> > +                               tag_clear_highpage(page + i);
> > +
> > +                       init = false;
>
> I find this a bit twisted and prone to breakages.
> Maybe just check for (init && !init_tags) below?

I did it this way deliberately. Check out the code after all the changes:

https://github.com/xairy/linux/blob/up-kasan-vmalloc-tags-v1/mm/page_alloc.c#L2447

It's possible to remove resetting the init variable by expanding the
if (init) check listing all conditions under which init is currently
reset, but that would essentially be duplicating the checks. I think
resetting init is more clear.

Please let me know what you think.

Thanks!