linux-kernel - Re: [tip: x86/urgent] x86/setup: Always reserve the first 1M of RAM

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: <ZAB/b+FjHjuRqe/S@zn.tnic>
Date:   Thu, 2 Mar 2023 11:50:33 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Hugh Dickins <hughd@...gle.com>,
        the arch/x86 maintainers <x86@...nel.org>
Subject: Re: [tip: x86/urgent] x86/setup: Always reserve the first 1M of RAM

On Wed, Mar 01, 2023 at 07:51:43PM -0800, Andy Lutomirski wrote:
> This is quite broken.  The comments in the patch seem to understand
> that Linux tries twice to allocate the real mode trampoline, but the
> code has some issues.
> 
> First, it actively breaks the logic here:
> 
> +               /*
> +                * Don't free memory under 1M for two reasons:
> +                * - BIOS might clobber it
> +                * - Crash kernel needs it to be reserved
> +                */
> +               if (start + size < SZ_1M)
> +                       continue;
> +               if (start < SZ_1M) {
> +                       size -= (SZ_1M - start);
> +                       start = SZ_1M;
> +               }
> +

Are you refering, per-chance, here to your comment in that same function
a bit higher?

Introduced by this thing here:

5bc653b73182 ("x86/efi: Allocate a trampoline if needed in efi_free_boot_services()")

?

Also, it looks like Mike did pay attention to your commit:

https://lore.kernel.org/all/YLZsEaimyAe0x6b3@kernel.org/

And then there's the whole deal with kdump kernel needing lowmem. The
function which became obsolete and got removed by:

23721c8e92f7 ("x86/crash: Remove crash_reserve_low_1M()")

So, considering how yours is the only report that breaks booting and
this reservation of <=1M has been out there for ~2 years without any
complaints, I'm thinking what we should do now is fix that logic.

Btw, this whole effort started with

  a799c2bd29d1 ("x86/setup: Consolidate early memory reservations")

Also see this:

ec35d1d93bf8 ("x86/setup: Document that Windows reserves the first MiB")

and with shit like that, we're "piggybacking" on Windoze since there
certification happens at least.

Which begs the question: how does your laptop even boot on windoze if
windoze reserves that 1M too?!

> I real the commit message and the linked bug, and I'm having trouble
> finding evidence of anything actually fixed by this patch.  Can we
> just revert it?  If not, it would be nice to get a fixup patch that
> genuinely cleans this up -- the whole structure of the code (first,
> try to allocate trampoline, then free boot services, then try again)
> isn't really conducive to a model where we *don't* free boot services
> < 1M.

Yes, I think this makes most sense. And that whole area is a minefield
so the less we upset the current universe, the better.

> Discovered by my delightful laptop, which does not boot with this patch applied.

How come your laptop hasn't booted new Linux since then?!? Tztztztz

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette