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 <b...@redhat.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 > > > sta...@vger.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 Later, Coiby got the root cause from investigation, please see: https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u > > Also, referencing the commit would be good. I assume this is discussing > commit 15fcedd43a08 ("kexec: Annotate struct crash_mem with __counted_by")? Right. > > > In the case exposed during reviewing this patch, the code UBSAN warned > > is not risky. > > Given that this makes things work correctly with newer compilers, I > would say it should be backported to whatever -stable kernels have the > "counted_by" annotation. (Hence the request to add a "Fixes" line so > that it will happen automatically.) Got it, then Coiby can post a standalone patch to fix commit 15fcedd43a08 ("kexec: Annotate struct crash_mem with __counted_by") and CC stable, then post a new version of this patch on top. Thanks a lot for confirming.
