linux-kernel - Re: [PATCH v4 06/14] kexec: Add KHO parsing support

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: <Z88ccZat5wm_iXx1@kernel.org>
Date: Mon, 10 Mar 2025 19:08:01 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Pratyush Yadav <ptyadav@...zon.de>
Cc: linux-kernel@...r.kernel.org, Alexander Graf <graf@...zon.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Lutomirski <luto@...nel.org>,
	Anthony Yznaga <anthony.yznaga@...cle.com>,
	Arnd Bergmann <arnd@...db.de>, Ashish Kalra <ashish.kalra@....com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Borislav Petkov <bp@...en8.de>,
	Catalin Marinas <catalin.marinas@....com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Eric Biederman <ebiederm@...ssion.com>,
	Ingo Molnar <mingo@...hat.com>, James Gowans <jgowans@...zon.com>,
	Jonathan Corbet <corbet@....net>,
	Krzysztof Kozlowski <krzk@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Pasha Tatashin <pasha.tatashin@...een.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Rob Herring <robh+dt@...nel.org>, Rob Herring <robh@...nel.org>,
	Saravana Kannan <saravanak@...gle.com>,
	Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Tom Lendacky <thomas.lendacky@....com>,
	Usama Arif <usama.arif@...edance.com>,
	Will Deacon <will@...nel.org>, devicetree@...r.kernel.org,
	kexec@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
	linux-doc@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org
Subject: Re: [PATCH v4 06/14] kexec: Add KHO parsing support

Hi Pratyush,

On Mon, Mar 10, 2025 at 04:20:01PM +0000, Pratyush Yadav wrote:
> Hi Mike,
> 
> On Thu, Feb 06 2025, Mike Rapoport wrote:
> [...]
> > @@ -444,7 +576,141 @@ static void kho_reserve_scratch(void)
> >  	kho_enable = false;
> >  }
> >  
> > +/*
> > + * Scan the DT for any memory ranges and make sure they are reserved in
> > + * memblock, otherwise they will end up in a weird state on free lists.
> > + */
> > +static void kho_init_reserved_pages(void)
> > +{
> > +	const void *fdt = kho_get_fdt();
> > +	int offset = 0, depth = 0, initial_depth = 0, len;
> > +
> > +	if (!fdt)
> > +		return;
> > +
> > +	/* Go through the mem list and add 1 for each reference */
> > +	for (offset = 0;
> > +	     offset >= 0 && depth >= initial_depth;
> > +	     offset = fdt_next_node(fdt, offset, &depth)) {
> > +		const struct kho_mem *mems;
> > +		u32 i;
> > +
> > +		mems = fdt_getprop(fdt, offset, "mem", &len);
> > +		if (!mems || len & (sizeof(*mems) - 1))
> > +			continue;
> > +
> > +		for (i = 0; i < len; i += sizeof(*mems)) {
> > +			const struct kho_mem *mem = &mems[i];
> 
> i goes from 0 to len in steps of 16, but you use it to dereference an
> array of type struct kho_mem. So you end up only looking at only one of
> every 16 mems and do an out of bounds access. I found this when testing
> the memfd patches and any time the file was more than 1 page, it started
> to crash randomly.

Thanks! Changyuan already pointed that out privately.
But I'm going to adopt the memory reservation scheme Jason proposed so
this code is going to go away anyway :)

> Below patch should fix that:
> 
> ---- 8< ----
> diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
> index c26753d613cbc..40d1d8ac68d44 100644
> --- a/kernel/kexec_handover.c
> +++ b/kernel/kexec_handover.c
> @@ -685,13 +685,15 @@ static void kho_init_reserved_pages(void)
>              offset >= 0 && depth >= initial_depth;
>              offset = fdt_next_node(fdt, offset, &depth)) {
>                 const struct kho_mem *mems;
> -               u32 i;
> +               u32 i, nr_mems;
>  
>                 mems = fdt_getprop(fdt, offset, "mem", &len);
>                 if (!mems || len & (sizeof(*mems) - 1))
>                         continue;
>  
> -               for (i = 0; i < len; i += sizeof(*mems)) {
> +               nr_mems = len / sizeof(*mems);
> +
> +               for (i = 0; i < nr_mems; i++) {
>                         const struct kho_mem *mem = &mems[i];
>  
>                         memblock_reserve(mem->addr, mem->size);
> ---- >8 ----
> [...]
> 
> -- 
> Regards,
> Pratyush Yadav

-- 
Sincerely yours,
Mike.