linux-kernel - Re: [PATCH v2] arch/x86: Check validity of EBDA pointer in mpparse.c

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] [day] [month] [year] [list]

Message-ID: <Ytly5ZArTaVAIKNq@czspare1-lap.sysgo.cz>
Date:   Thu, 21 Jul 2022 17:38:13 +0200
From:   Vit Kabele <vit.kabele@...go.com>
To:     Borislav Petkov <bp@...en8.de>, Vit Kabele <vit@...ele.me>
Cc:     platform-driver-x86@...r.kernel.org, r.marek@...embler.cz,
        x86@...nel.org, linux-kernel@...r.kernel.org, rafael@...nel.org,
        mingo@...hat.com
Subject: Re: [PATCH v2] arch/x86: Check validity of EBDA pointer in mpparse.c

On Tue, May 17, 2022 at 09:21:57PM +0200, Borislav Petkov wrote:
> Why can't default_find_smp_config() simply scan the last KiB below
> 640KiB twice for the sake of simpler code?
The problem is not in scanning the last KiB twice. But when the
ebda start is between 639 and 640 KiB, we need to adjust the size of the
scan window like MIN(1024, 640 * 1024 - address), because we don't want
to bump into the memory above 640K.

This is obviously not a problem to do, but since we are talking about
+/- a few lines, I thought it is more readable like that.

> I.e., there needs to be a single get_bios_ebda() - the current one can
> be renamed to __get_bios_ebda() - and that get_bios_ebda() should give
> either a sane EBDA address or 0 if the checks don't pass. And all code
> should use that.
I can do that if you consider it better, but it is a little bit more
lines and some code has to be duplicated.  E.g. the
reserve_bios_regions() cares about MIN(bios_start, ebda_start), so it
needs to read the BIOS_RAM_SIZE_KB_PTR and check its sanity anyway.
Since this is basically the code that would be carved out to the new
get_bios_ebda() helper, we don't save anything.

-- 
Best regards,
Vit Kabele