linux-kernel - Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

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: <g3zehuem3e4qqh3g7qfiquv2iwe52xo5z2pwrq3vaip2cpjche@nyjw4xbsf6vc>
Date: Tue, 20 May 2025 17:50:42 +0800
From: Coiby Xu <coxu@...hat.com>
To: Kees Cook <kees@...nel.org>, Baoquan He <bhe@...hat.com>
Cc: 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 Mon, May 19, 2025 at 09:22:30AM +0800, Baoquan He wrote:
>On 05/16/25 at 04:20pm, Kees Cook wrote:
[...]
>> > > 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
>
>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.

Yes, thank Kees for the confirmation and thank Baoquan for providing the
context and links! I'll send a standalone patch referencing
15fcedd43a08. But I don't think commit 15fcedd43a08 itself introduced
any bug so I shouldn't assign a Fixes tag to it. It's commit
5849cdf8c120 ("x86/crash: Fix crash_setup_memmap_entries() out-of-bounds
access") which forgot to set crash_mem->max_nr_ranges.
crash_mem->max_nr_ranges should always be set to make sure
crash_exclude_mem_range will work as expected. If
crash_mem->max_nr_ranges=0, crash_exclude_mem_range will return -ENOMEM
if there a range split. So if there is no objection, I will include 
   Fixes: 5849cdf8c120 ("x86/crash: Fix crash_setup_memmap_entries() out-of-bounds access")

A preview of to-be-sent patch is available via 
https://github.com/torvalds/linux/commit/43c5a68f3d01b2e065cbb8686279224710cba682

-- 
Best regards,
Coiby