linux-kernel - Re: [PATCH v7 14/18] x86/boot: make sure KASLR does not step over KHO preserved memory

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: <aBU2JQRKXjqoFClQ@kernel.org>
Date: Sat, 3 May 2025 00:16:21 +0300
From: Mike Rapoport <rppt@...nel.org>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Changyuan Lyu <changyuanl@...gle.com>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, anthony.yznaga@...cle.com, arnd@...db.de,
	ashish.kalra@....com, benh@...nel.crashing.org, bp@...en8.de,
	catalin.marinas@....com, corbet@....net,
	dave.hansen@...ux.intel.com, devicetree@...r.kernel.org,
	dwmw2@...radead.org, ebiederm@...ssion.com, graf@...zon.com,
	hpa@...or.com, jgowans@...zon.com, kexec@...ts.infradead.org,
	krzk@...nel.org, linux-arm-kernel@...ts.infradead.org,
	linux-doc@...r.kernel.org, linux-mm@...ck.org, luto@...nel.org,
	mark.rutland@....com, mingo@...hat.com, pasha.tatashin@...een.com,
	pbonzini@...hat.com, peterz@...radead.org, ptyadav@...zon.de,
	robh@...nel.org, rostedt@...dmis.org, saravanak@...gle.com,
	skinsburskii@...ux.microsoft.com, tglx@...utronix.de,
	thomas.lendacky@....com, will@...nel.org, x86@...nel.org
Subject: Re: [PATCH v7 14/18] x86/boot: make sure KASLR does not step over
 KHO preserved memory

On Fri, May 02, 2025 at 11:48:54AM -0700, Dave Hansen wrote:
> On 5/1/25 15:54, Changyuan Lyu wrote:
> > +/*
> > + * If KHO is active, only process its scratch areas to ensure we are not
> > + * stepping onto preserved memory.
> > + */
> > +#ifdef CONFIG_KEXEC_HANDOVER
> > +static bool process_kho_entries(unsigned long minimum, unsigned long image_size)
> > +{
> 
> I thought we agreed to rework this to unconditionally define the
> kho_scratch structures so the #ifdef can go away?

It's either #ifdef or double casting and my understanding was that your
preference was to get rid of the double casting.
 
> > +	struct kho_scratch *kho_scratch;
> > +	struct setup_data *ptr;
> > +	int i, nr_areas = 0;
> > +
> > +	ptr = (struct setup_data *)boot_params_ptr->hdr.setup_data;
> > +	while (ptr) {
> > +		if (ptr->type == SETUP_KEXEC_KHO) {
> > +			struct kho_data *kho = (struct kho_data *)ptr->data;
> > +
> > +			kho_scratch = (void *)kho->scratch_addr;
> > +			nr_areas = kho->scratch_size / sizeof(*kho_scratch);
> > +
> > +			break;
> > +		}
> > +
> > +		ptr = (struct setup_data *)ptr->next;
> > +	}
> > +
> > +	if (!nr_areas)
> > +		return false;
> > +
> > +	for (i = 0; i < nr_areas; i++) {
> > +		struct kho_scratch *area = &kho_scratch[i];
> > +		struct mem_vector region = {
> > +			.start = area->addr,
> > +			.size = area->size,
> > +		};
> > +
> > +		if (process_mem_region(&region, minimum, image_size))
> > +			break;
> > +	}
> > +
> > +	return true;
> > +}
> > +#else
> > +static inline bool process_kho_entries(unsigned long minimum,
> > +				       unsigned long image_size)
> > +{
> > +	return false;
> > +}
> > +#endif
> > +
> >  static unsigned long find_random_phys_addr(unsigned long minimum,
> >  					   unsigned long image_size)
> >  {
> > @@ -775,7 +824,8 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
> >  		return 0;
> >  	}
> >  
> > -	if (!process_efi_entries(minimum, image_size))
> > +	if (!process_kho_entries(minimum, image_size) &&
> > +	    !process_efi_entries(minimum, image_size))
> >  		process_e820_entries(minimum, image_size);
> >  
> >  	phys_addr = slots_fetch_random();
> 
> I made a comment about this in the last round, making this the second
> thing that I've noticed that was not addressed.
> 
> Could you please go back through the last round of comments before you
> repost these?

I presumed that changelog covers it. We'll add a comment here for the next
posting.
 
> Just to be clear: these are making progress, but they're not OK from the
> x86 side yet.

-- 
Sincerely yours,
Mike.