[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aCtBf2LqRqlWXaUp@MiWiFi-R3L-srv>
Date: Mon, 19 May 2025 22:34:39 +0800
From: Baoquan He <bhe@...hat.com>
To: Kees Cook <kees@...nel.org>
Cc: Coiby Xu <coxu@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>,
fuqiang wang <fuqiang.wang@...ystack.cn>,
Vivek Goyal <vgoyal@...hat.com>, Dave Young <dyoung@...hat.com>,
kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
x86@...nel.org
Subject: Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds
On 05/19/25 at 07:19am, Kees Cook wrote:
> On Mon, May 19, 2025 at 09:22:30AM +0800, Baoquan He wrote:
> > On 05/16/25 at 04:20pm, Kees Cook wrote:
> > > On Fri, May 16, 2025 at 11:35:12AM +0800, Baoquan He wrote:
> > > > On 05/11/25 at 10:19am, Coiby Xu wrote:
> > > > > On Fri, May 09, 2025 at 06:35:18PM -0700, Andrew Morton wrote:
> > > > > > On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@...hat.com> wrote:
> > > > > >
> > > > > > > > The bad commit was introduced in 2021 but only recent gcc-15 supports
> > > > > > > > __counted_by. That's why we don't see this UBSAN warning until this
> > > > > > > > year. And although this UBSAN warning is scary enough, fortunately it
> > > > > > > > doesn't cause a real problem.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Baoquan, please re-review this?
> > > > > > > > >
> > > > > > > > > A -stable backport is clearly required. A Fixes: would be nice, but I
> > > > > > > > > assume this goes back a long time so it isn't worth spending a lot of
> > > > > > > > > time working out when this was introduced.
> > > > > > > >
> > > > > > > > So I believe the correct fix should be as follows,
> > > > > > >
> > > > > > > Thanks for testing and investigation into these. Could you arrange this
> > > > > > > into formal patches based on your testing and analysis?
> > > > > > >
> > > > > > > It would be great if you can include Fuqiang's patch since it has
> > > > > > > conflict with your LUKS patch. This can facilitate patch merging for
> > > > > > > Andrew. Thanks in advance.
> > > > > >
> > > > > > Yes please, I'm a bit lost here.
> > > > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
> > > > > > presently in mm.git and I'd appreciate clarity on how to resolve the
> > > > > > conflicts which a new version of
> > > > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.
> > > > >
> > > > > I'll resolve any conflict between these patches. Before that, I'm not sure
> > > > > if a separate patch to fix the UBSAN warnings alone is needed to Cc
> > > > > stable@...r.kernel.org because 1) the UBSAN warnings don't mean there is a
> > > > > real problem;
> > > > > 2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
> > > > > warnings as a by-product.
> > > > >
> > > > > It seems the answer largely depends on if the stable tree or longterm
> > > > > trees need it. Currently, only longterm tree 6.12.28 and the stable tree
> > > > > 6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
> > > > > clang-18. Any advice will be appreciated! Thanks!
> > > >
> > > > I personally think UBSAN warning fix is not necessary for stable kernel.
> > > >
> > > > Hi Kees, Andrew,
> > > >
> > > > Could you help answer Coiby's question about whether we need post a
> > > > standalone patch to fix the UBSAN warning fix so that it can be back
> > > > ported to stable kernel?
> > >
> > > I went back through the thread and the referenced threads and I can't
> > > find any details on the USBAN splat. Can that please get reproduced in a
> > > commit log? That would help understand if it's a false positive or not.
> >
> >
> > The original patch is trying to fix a potential issue in which a memory
> > range is split, while the sub-range split out is always on top of the
> > entire memory range, hence no risk.
> >
> > Later, we encountered a UBSAN warning around the above memory range
> > splitting code several times. We found this patch can mute the warning.
> >
> > Please see below UBSAN splat trace report from Coiby:
> > https://lore.kernel.org/all/4de3c2onosr7negqnfhekm4cpbklzmsimgdfv33c52dktqpza5@z5pb34ghz4at/T/#u
>
> Ah-ha! Thanks for the link.
>
> > Later, Coiby got the root cause from investigation, please see:
> > https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u
>
> Looking at https://lore.kernel.org/all/aBxfflkkQXTetmbq@MiWiFi-R3L-srv/
> it seems like this actually turned out to be a legitimate overflow
> detection? I.e. the fix isn't silencing a false positive, but rather
> allocating enough space?
This v5 is on top of below patch which Andrew has picked to his mm tree.
In there, it happened to get the ubsan warning fixed. But the hunk
doesn't reflect it in the v5 patch.
[PATCH v9 7/8] x86/crash: pass dm crypt keys to kdump kernel
https://lore.kernel.org/all/20250502011246.99238-8-coxu@redhat.com/T/#u
Powered by blists - more mailing lists