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: <Zs2daJ9FHA8vPkPP@MiWiFi-R3L-srv>
Date: Tue, 27 Aug 2024 17:33:28 +0800
From: Baoquan He <bhe@...hat.com>
To: Dave Young <dyoung@...hat.com>
Cc: Tom Lendacky <thomas.lendacky@....com>, linux-kernel@...r.kernel.org,
	noodles@...com, x86@...nel.org, lijiang@...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 08/27/24 at 01:41pm, Dave Young wrote:
> On Tue, 27 Aug 2024 at 13:28, Baoquan He <bhe@...hat.com> wrote:
> >
> > On 08/26/24 at 09:24am, Tom Lendacky wrote:
> > > On 8/25/24 21:44, Baoquan He wrote:
......
> > > > ---
> > > >  arch/x86/mm/ioremap.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > > > index aa7d279321ea..7953c4a1d28d 100644
> > > > --- a/arch/x86/mm/ioremap.c
> > > > +++ b/arch/x86/mm/ioremap.c
> > > > @@ -717,7 +717,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
> > > >             paddr_next = data->next;
> > > >             len = data->len;
> > > >
> > > > -           if ((phys_addr > paddr) && (phys_addr < (paddr + len))) {
> > > > +           if ((phys_addr > paddr) && (phys_addr < (paddr + size + len))) {
> > >
> > > I don't think this is correct. You are adding the requested size to the
> > > length of the setup data element. The length is the true length of the
> > > setup data and should not be increased.
> >
> > I talked to Dave, he reminded me that people could mix the passed in
> > parameter 'size' and the local variable 'size' defined inside the while
> > loop, not sure which 'size' you are referring to.
> >
> Baoquan, you are right, but I think I mistakenly read the code in
> memremap_is_setup_data instead of early_memremap_is_setup_data.  You
> can check the memremap_is_setup_data, no "size = sizeof (*data)",  so
> these two functions could both need fixes.

Agree, memremap_is_setup_data() has the same drawback in code.

> 
> Otherwise it would be better to change the function internal variable
> name, it could cause confusion even if the actual result is correct.

Ok, will consider to change when spinning v2.