linux-kernel - Re: [PATCH] x86/mm/sme: fix the kdump kernel breakage on SME system when CONFIG_IMA

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: <ac929075-27a3-8cbd-5ffd-966ea3e82697@amd.com>
Date: Tue, 27 Aug 2024 09:00:51 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Baoquan He <bhe@...hat.com>
Cc: linux-kernel@...r.kernel.org, noodles@...com, x86@...nel.org,
 lijiang@...hat.com, dyoung@...hat.com, kexec@...ts.infradead.org
Subject: Re: [PATCH] x86/mm/sme: fix the kdump kernel breakage on SME system
 when CONFIG_IMA_KEXEC=y

On 8/27/24 08:52, Tom Lendacky wrote:
> On 8/26/24 22:19, Baoquan He wrote:
>> On 08/26/24 at 09:24am, Tom Lendacky wrote:
>>> On 8/25/24 21:44, Baoquan He wrote:
>>>> Recently, it's reported that kdump kernel is broken during bootup on
>>>> SME system when CONFIG_IMA_KEXEC=y. When debugging, I noticed this
>>>> can be traced back to commit ("b69a2afd5afc x86/kexec: Carry forward
>>>> IMA measurement log on kexec"). Just nobody ever tested it on SME
>>>> system when enabling CONFIG_IMA_KEXEC.
>>>>
>>>>
>>>> Here fix the code bug to make kexec/kdump kernel boot up successfully.
>>>>
>>>> Fixes: 8f716c9b5feb ("x86/mm: Add support to access boot related data in the clear")
>>>
>>> The check that was modified was added by:
>>> 	b3c72fc9a78e ("x86/boot: Introduce setup_indirect")
>>>
>>> The SETUP_INDIRECT patches seem to be the issue here.
>>
>> Hmm, I didn't check it carefully, thanks for addding this info. While
>> after checking commit b3c72fc9a78e, I feel the adding code was trying to
>> fix your original early_memremap_is_setup_data(). Even though
>> SETUP_INDIRECT type of setup_data has been added, the original
>> early_memremap_is_setup_data() only check the starting address and
>> the content of struct setup_data, that's obviously wrong.
> 
> IIRC, when this function was created, the value of "len" in setup_data
> included the length of "data", so the calculation was correct. Everything
> was contiguous in a setup_data element.
> 
>>
>> arch/x86/include/uapi/asm/setup_data.h:
>> /* extensible setup data list node */
>> struct setup_data {
>>         __u64 next;
>>         __u32 type;
>>         __u32 len;
>>         __u8 data[];
>> };
>>
>> As you can see, the zero-length will embed the carried data which is
>> actually expected and adjacent to its carrier, the struct setup_data.
> 
> Right, and "len" is the length of that data. So paddr + len goes to the
> end of the overall setup_data.

Ah, I see what you're saying. "len" doesn't include the size of the
setup_data structure, only the data. If so, then, yes, adding a sizeof()
to the calculation in the if statement is correct.

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>
>>>