linux-kernel - Re: [PATCH] efi: don't map the entire mokvar table to determine its size

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: <CAMj1kXHrrQ_39528oEQ8szEBKPQp8+1V7SGsNSp0fbPV=RWFJA@mail.gmail.com>
Date: Thu, 27 Feb 2025 18:20:35 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Peter Jones <pjones@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-efi@...r.kernel.org, 
	Lenny Szubowicz <lszubowi@...hat.com>
Subject: Re: [PATCH] efi: don't map the entire mokvar table to determine its size

On Thu, 27 Feb 2025 at 18:13, Peter Jones <pjones@...hat.com> wrote:
>
> On Thu, Feb 27, 2025 at 08:50:08AM +0100, Ard Biesheuvel wrote:
> >
> > Should we add something like the below to avoid mapping the same page
> > over and over again? Or is this premature optimization?
> >
>
> I can't honestly say I'm sure either way, but I'm leaning towards
> thinking it's probably worthwhile.  On my development tree the number of
> these we wind up doing in the maximal case is 31, and but in a typical
> case it's more like 20, with a series that looks something like the
> below list of sizes and relative addresses from the first entry.  (I
> generated this with ls and awk, so it's not quite exact but it's
> fairly representative.)  I've marked which ones could be eliminated.
>
> map 264 at 0x0
> unmap 264 at 0x0    <-- gone
> map 264 at 0x146    <-- gone
> unmap 264 at 0x146  <-- gone
> map 264 at 0x2a2    <-- gone
> unmap 264 at 0x2a2  <-- gone
> map 264 at 0x43e    <-- gone
> unmap 264 at 0x43e  <-- gone
> map 264 at 0x548    <-- gone
> unmap 264 at 0x548  <-- gone
> map 264 at 0x660    <-- gone
> unmap 264 at 0x660  <-- gone
> map 264 at 0x84d    <-- gone
> unmap 264 at 0x84d
> map 264 at 0x191f
> unmap 264 at 0x191f <-- gone
> map 264 at 0x1a73   <-- gone
> unmap 264 at 0x1a73 <-- gone
> map 264 at 0x1b7c   <-- gone
> unmap 264 at 0x1b7c <-- gone
> map 264 at 0x1cd0   <-- gone
> unmap 264 at 0x1cd0
> map 264 at 0x21a8
> unmap 264 at 0x21a8 <-- gone
> map 264 at 0x22c2   <-- gone
> unmap 264 at 0x22c2 <-- gone
> map 264 at 0x23cb   <-- gone
> unmap 264 at 0x23cb <-- gone
> map 264 at 0x24d4   <-- gone
> unmap 264 at 0x24d4 <-- gone
> map 264 at 0x263c   <-- gone
> unmap 264 at 0x263c <-- gone
> map 264 at 0x2746   <-- gone
> unmap 264 at 0x2746
> map 264 at 0x4043
> unmap 264 at 0x4043
> map 264 at 0x86f7
> unmap 264 at 0x86f7
>
> So going from 19 map/unmap pairs to 5.  Seems like it can't hurt, but
> it's a small number either way.
>

So this would go from 19 to 3 on a 16k pages kernel. So I'm leaning to
applying it as well.

BTW these results confirm my suspicion that these headers may appear
misaligned, hence the __aligned(1)

> Anyway, I tried your patch and it works for me:
>
> Tested-By: Peter Jones <pjones@...hat.com>
>

Thanks. I'll apply it as a separate patch, and only tag your patch cc:stable