Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 8 Sep 2023 14:50:01 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Zhizhou Zhang <zhizhou.zh@gmail.com>, rppt@kernel.org,
        akpm@linux-foundation.org, vishal.moola@gmail.com, arnd@arndb.de,
        wangkefeng.wang@huawei.com, willy@infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Zhizhou Zhang <zhizhouzhang@asrmicro.com>
Subject: Re: [PATCH] ARM: mm: fix stack corruption when CONFIG_ARM_PV_FIXUP=y
Message-ID: <ZPsmiV9I4qbqpAsO@shell.armlinux.org.uk>
References: <20230907143302.4940-1-zhizhou.zh@gmail.com>
 <CACRpkdY=EYQfRrUaw5JL5e0x2p=KurYfwvfunJ+GF1PmupKNqg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CACRpkdY=EYQfRrUaw5JL5e0x2p=KurYfwvfunJ+GF1PmupKNqg@mail.gmail.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

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@gmail.com> wrote:
> 
> > From: Zhizhou Zhang <zhizhouzhang@asrmicro.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@asrmicro.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I would also add
> Cc: stable@vger.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!