linux-kernel - Re: [PATCH 2/2 v2] x86/e820: Use much less memory for e820/e820

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]

Date:   Thu, 15 Sep 2016 09:04:30 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Denys Vlasenko <dvlasenk@...hat.com>
Cc:     Andy Lutomirski <luto@...capital.net>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Brian Gerst <brgerst@...il.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2 v2] x86/e820: Use much less memory for
 e820/e820_saved, save up to 120k


* Denys Vlasenko <dvlasenk@...hat.com> wrote:

> The maximum size of e820 map array for EFI systems is defined as
> E820_X_MAX (E820MAX + 3 * MAX_NUMNODES).
> 
> In x86_64 defconfig, this ends up with E820_X_MAX = 320, e820 and e820_saved
> are 6404 bytes each.
> 
> With larger configs, for example Fedora kernels, E820_X_MAX = 3200, e820 and e820_saved
> are 64004 bytes each. Most of this space is wasted. Typical machines have some 20-30
> e820 areas at most.
> 
> This patch turns e820 and e820_saved to pointers which initially point to __initdata
> tables, of the same size as before.
> 
> At the very end of setup_arch(), when we are done fiddling with these maps,
> allocate smaller alloc_bootmem blocks, copy maps there, and change pointers.
> 
> Run-tested.

> +/*
> + * Initial e820 and e820_saved are largish __initdata arrays.
> + * Copy them to (usually much smaller) dynamically allocated area.
> + * This is done after all tweaks we ever do to them.
> + */
> +__init void e820_reallocate_tables(void)
> +{
> +	struct e820map *n;
> +	int size;
> +
> +	size = offsetof(struct e820map, map) + sizeof(struct e820entry) * e820->nr_map;
> +	n = alloc_bootmem(size);
> +	memcpy(n, e820, size);
> +	e820 = n;
> +
> +	size = offsetof(struct e820map, map) + sizeof(struct e820entry) * e820_saved->nr_map;
> +	n = alloc_bootmem(size);
> +	memcpy(n, e820_saved, size);
> +	e820_saved = n;
> +}

Ok, this makes me quite nervous, could you please split this into two patches so 
that any fails can be nicely bisected to?

First patch only does the pointerization changes with a trivial placeholder 
structure (full size, static allocated), second patch does all the dangerous bits 
such as changing it to __initdata, allocating and copying over bits.

Also, could we please also add some minimal debugging facility to make sure the 
memory table does not get extended after it's been reallocated?

Thanks,

	Ingo