linux-kernel - Re: [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned memory reservations

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: <CAMj1kXED+Ry7FEtZm-5tL5ZD2c4+nbYtCWrzQmd158+jXHNqPQ@mail.gmail.com>
Date:   Sat, 31 Oct 2020 12:21:24 +0100
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     linux-xtensa@...ux-xtensa.org,
        Florian Fainelli <f.fainelli@...il.com>,
        Chris Zankel <chris@...kel.net>,
        Linus Walleij <linus.walleij@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Max Filippov <jcmvbkbc@...il.com>,
        Mike Rapoport <rppt@...nel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] ARM, xtensa: highmem: avoid clobbering non-page aligned
 memory reservations

On Sat, 31 Oct 2020 at 12:04, Russell King - ARM Linux admin
<linux@...linux.org.uk> wrote:
>
> On Sat, Oct 31, 2020 at 11:47:42AM +0100, Ard Biesheuvel wrote:
> > On Sat, 31 Oct 2020 at 11:33, Russell King - ARM Linux admin
> > <linux@...linux.org.uk> wrote:
> > >
> > > On Sat, Oct 31, 2020 at 11:43:45AM +0200, Mike Rapoport wrote:
> > > > From: Ard Biesheuvel <ardb@...nel.org>
> > > >
> > > > free_highpages() iterates over the free memblock regions in high
> > > > memory, and marks each page as available for the memory management
> > > > system.
> > > >
> > > > Until commit cddb5ddf2b76 ("arm, xtensa: simplify initialization of
> > > > high memory pages") it rounded beginning of each region upwards and end of
> > > > each region downwards.
> > > >
> > > > However, after that commit free_highmem() rounds the beginning and end of
> > > > each region downwards, and we may end up freeing a page that is
> > > > memblock_reserve()d, resulting in memory corruption.
> > > >
> > > > Restore the original rounding of the region boundaries to avoid freeing
> > > > reserved pages.
> > > >
> > > > Fixes: cddb5ddf2b76 ("arm, xtensa: simplify initialization of high memory pages")
> > > > Link: https://lore.kernel.org/r/20201029110334.4118-1-ardb@kernel.org/
> > > > Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> > > > Co-developed-by:  Mike Rapoport <rppt@...ux.ibm.com>
> > > > Signed-off-by: Mike Rapoport <rppt@...ux.ibm.com>
> > > > ---
> > > >
> > > > Max, Russell,
> > > >
> > > > Please let me know how do you prefer to take it upstream.
> > > > If needed this can go via memblock tree.
> > > >
> > > > v2: fix words order in the commit message
> > >
> > > I really don't understand what is going on here; there seems to be a
> > > total disconnect of communication between yourself and Ard. Ard has
> > > already submitted a different patch for this to the patch system
> > > already, sent yesterday.
> > >
> > > https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9021/1
> > >
> > > Please discuss between yourselves how you want to solve the problem,
> > > and then submit an agreed and tested patch to those of us upstream;
> > > please don't make it for those upstream to pick one of your patches
> > > as you are at present.
> > >
> >
> > Apologies for creating this confusion. I posted a patch and dropped it
> > into the patch system when I found the bug.
> >
> > However, only when Florian asked about a 'fixes' tag, I went back to
> > the history, and realized that the issue was introduced by Mike during
> > the most recent merge window, and affects xtensa as well.
>
> So why does Mike's patch have:
>
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
>
> in it? It seems you haven't been directly involved in Mike's patch.
>

Because I cc'ed him on the discussion following the patch that is now
in your patch system. So he took that patch and modified it, but
retained the original S-o-b and authorship.

> There's something /really/ not right with the process behind this
> patch.
>
> > I don't have a preference which patch gets applied, though, so please
> > indicate your preference, and we will adapt accordingly.
>
> I asked for you both to come to a concensus about how you want to
> proceed, and now you're throwing it back on to me to solve your(pl)
> mis-communication issue. We haven't heard from Mike yet.
>

I am not throwing it back to you. I merely indicated that I have no
preference, and since Mike is the one who introduced this issue in the
first place, I am expecting him to drive this. And indeed, we haven't
heard from him yet.

> Clearly, I wasn't blunt and stroppy enough to be properly understood.
> Sort it out between yourselves and tell me which patch you want me to
> apply.
>

I would like you to ack this version of the patch, and disregard the
one in the patch system, so that Mike can take this one through the
memblock tree where the issue originated in the first place.