[PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Michal Clapinski]

The intent of the code was to cancel KASLR if there are more than 4
memmap args. Unfortunately, it was only doing that if the memmap args
were comma delimited, not if they were entirely separate.

So it would disable physical KASLR for:
memmap=1G!4G,1G!5G,1G!6G,1G!7G,1G!8G
since the whole function is just called once and we hit the `if` at
the end of the function.

But it would not disable physical KASLR for:
memmap=1G!4G memmap=1G!5G memmap=1G!6G memmap=1G!7G memmap=1G!8G
The whole function would be called 5 times in total. On the 4th run the
last `if` would not trigger since `str` would be null. On the 5th run
we would exit early via the first `if`. That way the last `if` would
never trigger.

For the second input, the code would avoid the first 4 memmap regions
but not the last one (it could put the kernel there).

The new code disables physical KASLR for both of those inputs.

Signed-off-by: Michal Clapinski <mclapinski@google.com>
Suggested-by: Chris Li <chrisl@kernel.org>
Link: https://lore.kernel.org/all/CAF8kJuMvX31n8yNWn11bo1wCgXXOwOAp8HbYpSEBy94LR6phDA@mail.gmail.com/
Fixes: d52e7d5a952c ("x86/KASLR: Parse all 'memmap=' boot option entries")
---
v3: added a link in the description and a better explanation
v2: used Chris Li's snippet to change the flow of the function
---
 arch/x86/boot/compressed/kaslr.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 3b0948ad449f..649264503ce6 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -162,14 +162,18 @@ static void mem_avoid_memmap(char *str)
 {
 	static int i;
 
-	if (i >= MAX_MEMMAP_REGIONS)
-		return;
-
-	while (str && (i < MAX_MEMMAP_REGIONS)) {
+	while (str) {
 		int rc;
 		u64 start, size;
-		char *k = strchr(str, ',');
+		char *k;
+
+		if (i >= MAX_MEMMAP_REGIONS) {
+			/* Too many memmap regions, disable physical KASLR. */
+			memmap_too_large = true;
+			return;
+		}
 
+		k = strchr(str, ',');
 		if (k)
 			*k++ = 0;
 
@@ -190,10 +194,6 @@ static void mem_avoid_memmap(char *str)
 		mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].size = size;
 		i++;
 	}
-
-	/* More than 4 memmaps, fail kaslr */
-	if ((i >= MAX_MEMMAP_REGIONS) && str)
-		memmap_too_large = true;
 }
 
 /* Store the number of 1GB huge pages which users specified: */
-- 
2.51.2.1026.g39e6a42477-goog

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Ard Biesheuvel]

On Thu, 6 Nov 2025 at 17:23, Michal Clapinski <mclapinski@google.com> wrote:
>
> The intent of the code was to cancel KASLR if there are more than 4
> memmap args. Unfortunately, it was only doing that if the memmap args
> were comma delimited, not if they were entirely separate.
>
> So it would disable physical KASLR for:
> memmap=1G!4G,1G!5G,1G!6G,1G!7G,1G!8G
> since the whole function is just called once and we hit the `if` at
> the end of the function.
>
> But it would not disable physical KASLR for:
> memmap=1G!4G memmap=1G!5G memmap=1G!6G memmap=1G!7G memmap=1G!8G
> The whole function would be called 5 times in total. On the 4th run the
> last `if` would not trigger since `str` would be null. On the 5th run
> we would exit early via the first `if`. That way the last `if` would
> never trigger.
>
> For the second input, the code would avoid the first 4 memmap regions
> but not the last one (it could put the kernel there).
>
> The new code disables physical KASLR for both of those inputs.
>
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> Suggested-by: Chris Li <chrisl@kernel.org>
> Link: https://lore.kernel.org/all/CAF8kJuMvX31n8yNWn11bo1wCgXXOwOAp8HbYpSEBy94LR6phDA@mail.gmail.com/
> Fixes: d52e7d5a952c ("x86/KASLR: Parse all 'memmap=' boot option entries")
> ---
> v3: added a link in the description and a better explanation
> v2: used Chris Li's snippet to change the flow of the function
> ---
>  arch/x86/boot/compressed/kaslr.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>

This looks fine to me, but as I noted before, I'd happily simply
disable physical KASLR entirely if 'memmap=' appears at all.

> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 3b0948ad449f..649264503ce6 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -162,14 +162,18 @@ static void mem_avoid_memmap(char *str)
>  {
>         static int i;
>
> -       if (i >= MAX_MEMMAP_REGIONS)
> -               return;
> -
> -       while (str && (i < MAX_MEMMAP_REGIONS)) {
> +       while (str) {
>                 int rc;
>                 u64 start, size;
> -               char *k = strchr(str, ',');
> +               char *k;
> +
> +               if (i >= MAX_MEMMAP_REGIONS) {
> +                       /* Too many memmap regions, disable physical KASLR. */
> +                       memmap_too_large = true;
> +                       return;
> +               }
>
> +               k = strchr(str, ',');
>                 if (k)
>                         *k++ = 0;
>
> @@ -190,10 +194,6 @@ static void mem_avoid_memmap(char *str)
>                 mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].size = size;
>                 i++;
>         }
> -
> -       /* More than 4 memmaps, fail kaslr */
> -       if ((i >= MAX_MEMMAP_REGIONS) && str)
> -               memmap_too_large = true;
>  }
>
>  /* Store the number of 1GB huge pages which users specified: */
> --
> 2.51.2.1026.g39e6a42477-goog
>

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Dave Hansen]

On 11/6/25 08:32, Ard Biesheuvel wrote:
> This looks fine to me, but as I noted before, I'd happily simply
> disable physical KASLR entirely if 'memmap=' appears at all.

Hey, post the patch and we'll see which one looks better.

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Michał Cłapiński]

On Thu, Nov 6, 2025 at 5:36 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 11/6/25 08:32, Ard Biesheuvel wrote:
> > This looks fine to me, but as I noted before, I'd happily simply
> > disable physical KASLR entirely if 'memmap=' appears at all.
>
> Hey, post the patch and we'll see which one looks better.

Ard's solution [1] hasn't been merged anywhere AFAIK. The bug persists.

Can we merge my solution to fix the issue sooner rather than later?
Merging my code won't prevent Ard's simplification from being merged
in the future if the maintainers so choose.

[1] https://lore.kernel.org/all/20251106173019.1224443-2-ardb+git@google.com/

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Borislav Petkov]

On Mon, Dec 08, 2025 at 02:27:54PM +0100, Michał Cłapiński wrote:
> Can we merge my solution to fix the issue sooner rather than later?

We are in such a hurry and during the merge window because?

Nothing's stopping you from merging your solution into your kernels in
the meantime.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Michał Cłapiński]

On Tue, Dec 9, 2025 at 1:44 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Dec 08, 2025 at 02:27:54PM +0100, Michał Cłapiński wrote:
> > Can we merge my solution to fix the issue sooner rather than later?
>
> We are in such a hurry and during the merge window because?

I'm sorry, I didn't realize it was the merge window. I'm new to this process.

I'm not in a hurry. What I meant is I understand that Ard's change
would also fix the issue but it's a bigger change with a higher chance
of being rolled back. That's why I believe it's a good idea to merge
my change first and then later merge Ard's change. This way, even if
it's rolled back, it won't make the bug reappear.

> Nothing's stopping you from merging your solution into your kernels in
> the meantime.

I have this solution in my kernel. I just don't want to carry it forever.

What should I do now? Should I change something in the code? Should I just wait?

Thanks for your help.

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Ard Biesheuvel]

On Mon, 29 Dec 2025 at 15:25, Michał Cłapiński <mclapinski@google.com> wrote:
>
> On Tue, Dec 9, 2025 at 1:44 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Mon, Dec 08, 2025 at 02:27:54PM +0100, Michał Cłapiński wrote:
> > > Can we merge my solution to fix the issue sooner rather than later?
> >
> > We are in such a hurry and during the merge window because?
>
> I'm sorry, I didn't realize it was the merge window. I'm new to this process.
>
> I'm not in a hurry. What I meant is I understand that Ard's change
> would also fix the issue but it's a bigger change with a higher chance
> of being rolled back. That's why I believe it's a good idea to merge
> my change first and then later merge Ard's change. This way, even if
> it's rolled back, it won't make the bug reappear.
>
> > Nothing's stopping you from merging your solution into your kernels in
> > the meantime.
>
> I have this solution in my kernel. I just don't want to carry it forever.
>
> What should I do now? Should I change something in the code? Should I just wait?
>

Feel free to resend, linking to all the relevant discussions that were
had on the matter (below the ---).

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Borislav Petkov]

On Mon, Dec 29, 2025 at 03:25:12PM +0100, Michał Cłapiński wrote:
> I'm not in a hurry. What I meant is I understand that Ard's change
> would also fix the issue but it's a bigger change with a higher chance
> of being rolled back.

Why is that so?

Ard's change is simply dropping those cmdline params which are conflicting
anyway.

So why is there even a risk to be reverted?

> What should I do now? Should I change something in the code? Should I just
> wait?

I think we should merge Ard's change directly and be done with it.

Unless I'm missing an angle...?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Michał Cłapiński]

On Mon, Jan 12, 2026 at 10:39 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Dec 29, 2025 at 03:25:12PM +0100, Michał Cłapiński wrote:
> > I'm not in a hurry. What I meant is I understand that Ard's change
> > would also fix the issue but it's a bigger change with a higher chance
> > of being rolled back.
>
> Why is that so?
>
> Ard's change is simply dropping those cmdline params which are conflicting
> anyway.

Currently, if you have 4 or fewer memmaps, KASLR correctly avoids
putting the kernel there. That just works now and Ard's change would
disable that.

> So why is there even a risk to be reverted?

Not because I expect something to break. You want to disable a
security measure that works currently (up to 4 memmaps). Some security
engineer might see this in rc1 and request a revert. My change doesn't
disable a security measure so I don't expect it to be reverted.

> > What should I do now? Should I change something in the code? Should I just
> > wait?
>
> I think we should merge Ard's change directly and be done with it.
>
> Unless I'm missing an angle...?

I'd be okay with that too (assuming it's not reverted).

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Borislav Petkov]

On Tue, Jan 13, 2026 at 12:21:25PM +0100, Michał Cłapiński wrote:
> Currently, if you have 4 or fewer memmaps, KASLR correctly avoids
> putting the kernel there. That just works now and Ard's change would
> disable that.

And why do we care about 4 or fewer memmaps? Is that a valid use case which we
have to support? 4 is magic but 5 is a no-no?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Michał Cłapiński]

On Tue, Jan 13, 2026 at 12:27 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Jan 13, 2026 at 12:21:25PM +0100, Michał Cłapiński wrote:
> > Currently, if you have 4 or fewer memmaps, KASLR correctly avoids
> > putting the kernel there. That just works now and Ard's change would
> > disable that.
>
> And why do we care about 4 or fewer memmaps? Is that a valid use case which we
> have to support? 4 is magic but 5 is a no-no?

I expect most people who use memmaps use 4 or fewer, which was the
reason given when this code was originally written.

But generally I agree. I expected my change to be merged quickly and I
wanted to propose a new change on top of that that would increase that
number (either to a very high number or just fully remove the
restriction).

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Borislav Petkov]

On Tue, Jan 13, 2026 at 12:39:01PM +0100, Michał Cłapiński wrote:
> I expect most people who use memmaps use 4 or fewer, which was the
> reason given when this code was originally written.
> 
> But generally I agree. I expected my change to be merged quickly and I
> wanted to propose a new change on top of that that would increase that
> number (either to a very high number or just fully remove the
> restriction).

Yes, and as you notice yourself, the question of how high that number should
be is hard to answer. Perhaps even non-sensical.

Why would one want to specify memmap *and* expect KASLR to be functioning
optimally after it has limitations imposed on the range?

Or, to put it differently, under which memmap= setting can KASLR still
function optimally and where does it get impaired? That could be one criterion
to use IMHO...

But ofc, if the memmap thing is only an "expectation", then simply zap it and
be done with it.

As Ard said, you could write a detailed commit message explaining the
rationale and then we can queue it and see who complains.

I'd say...

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v3 1/1] x86/boot/compressed: Fix avoiding memmap in physical KASLR

[Ard Biesheuvel]

On Tue, 13 Jan 2026 at 12:54, Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Jan 13, 2026 at 12:39:01PM +0100, Michał Cłapiński wrote:
> > I expect most people who use memmaps use 4 or fewer, which was the
> > reason given when this code was originally written.
> >
> > But generally I agree. I expected my change to be merged quickly and I
> > wanted to propose a new change on top of that that would increase that
> > number (either to a very high number or just fully remove the
> > restriction).
>
> Yes, and as you notice yourself, the question of how high that number should
> be is hard to answer. Perhaps even non-sensical.
>
> Why would one want to specify memmap *and* expect KASLR to be functioning
> optimally after it has limitations imposed on the range?
>
> Or, to put it differently, under which memmap= setting can KASLR still
> function optimally and where does it get impaired? That could be one criterion
> to use IMHO...
>
> But ofc, if the memmap thing is only an "expectation", then simply zap it and
> be done with it.
>
> As Ard said, you could write a detailed commit message explaining the
> rationale and then we can queue it and see who complains.
>

Lack of physical KASLR is not a big deal unless a) the physical
placement is predictable, and b) the linear map is not randomized
either, because that will result in linear aliases of kernel data
structures being predictable as well.

Note that since v6.6 (backported to v6.1), EFI boot via the stub no
longer calls into this code - instead, it performs a randomized
allocation using the EFI boot services, and places the kernel there,
unless it encounters any of mem=, memmap= or hugepages= on the command
line [0], in which case it allocates at the base of system RAM.

This is why I suggested to do the same here.


[0] 15aa8fb852f9 x86/efistub: Omit physical KASLR when memory reservations exist