linux-kernel - Re: [PATCH] ARM: mm: fix stack corruption when CONFIG_ARM_PV

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]

Date:   Fri, 8 Sep 2023 14:50:01 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Zhizhou Zhang <zhizhou.zh@...il.com>, rppt@...nel.org,
        akpm@...ux-foundation.org, vishal.moola@...il.com, arnd@...db.de,
        wangkefeng.wang@...wei.com, willy@...radead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Zhizhou Zhang <zhizhouzhang@...micro.com>
Subject: Re: [PATCH] ARM: mm: fix stack corruption when CONFIG_ARM_PV_FIXUP=y

On Fri, Sep 08, 2023 at 02:58:49PM +0200, Linus Walleij wrote:
> Hi Zhizhou,
> 
> wow a great patch! I'm surprised no-one has been hit by this before.
> I guess we were lucky.
> 
> On Thu, Sep 7, 2023 at 4:33 PM Zhizhou Zhang <zhizhou.zh@...il.com> wrote:
> 
> > From: Zhizhou Zhang <zhizhouzhang@...micro.com>
> >
> > flush_cache_all() save registers to stack at function entry.
> > If it's called after cache disabled, the data is written to
> > memory directly. So the following clean cache operation corrupted
> > registers saved by flush_cache_all(), including lr register.
> > calling flush_cache_all() before turn off cache fixed the problem.
> >
> > Signed-off-by: Zhizhou Zhang <zhizhouzhang@...micro.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> 
> I would also add
> Cc: stable@...r.kernel.org
> 
> Then please put this into Russell's patch tracker once review
> is complete.

However, it makes a total nonsense of the comment, which explains
precisely why the flush_cache_all() is where it is. Moving it before
that comment means that the comment is now rediculous.

So, please don't put it in the patch system.

The patch certainly needs to be tested on TI Keystone which is the
primary user of this code.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!