x86/boot: Reject truncated acpi_rsdp= values

PATCH Superseded Thorsten Blum 2 participants 13 messages Jun 17 – Jun 19

Subsystems (2)

Stable branch x86

lore

Share & tools

lore

Thorsten Blum

author

2026-06-17 13:05 UTC #0 lore raw

PATCH 1/1 x86/boot: Reject truncated acpi_rsdp= values

cmdline_find_option() returns the full length of the argument value even if it is truncated. However, get_cmdline_acpi_rsdp() only checks whether acpi_rsdp= is present and does not reject truncated values that do not fit in the buffer.

Reject truncated values early to prevent boot_kstrtoul() from parsing a partial value and thus from silently using the wrong RSDP address.

Fixes 3c98e71b42a7 ("x86/boot: Add "acpi_rsdp=" early parsing")

Cc [email protected]

Signed-off-by Thorsten Blum <[email protected]>

---
 arch/x86/boot/compressed/acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/x86/boot/compressed/acpi.c +2 -2

index f196b1d1ddf8..1b5638a8e180 100644
@@ -184,8 +184,8 @@ static unsigned long get_cmdline_acpi_rsdp(void)
 	char val[MAX_ADDR_LEN] = { };
 	int ret;
 
-	ret = cmdline_find_option("acpi_rsdp", val, MAX_ADDR_LEN);
-	if (ret < 0)
+	ret = cmdline_find_option("acpi_rsdp", val, sizeof(val));
+	if (ret < 0 || ret >= sizeof(val))
 		return 0;
 
 	if (boot_kstrtoul(val, 16, &addr))

Borislav Petkov

maintainer

2026-06-18 04:54 UTC #1 lore raw

Quoted 7 lines · Thorsten Blum

cmdline_find_option() returns the full length of the argument value even if it is truncated. However, get_cmdline_acpi_rsdp() only checks whether acpi_rsdp= is present and does not reject truncated values that do not fit in the buffer.
Reject truncated values early to prevent boot_kstrtoul() from parsing a partial value and thus from silently using the wrong RSDP address.

And?

If it uses the wrong address, it'll crash'n'burn later. As it should be.

Or are we protecting people from shooting themselves in foot now too?

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Thorsten Blum

author

2026-06-18 15:03 UTC #2 lore raw

Quoted 6 lines · Borislav Petkov

Quoted 5 lines · Thorsten Blum
cmdline_find_option() returns the full length of the argument value even if it is truncated. However, get_cmdline_acpi_rsdp() only checks whether acpi_rsdp= is present and does not reject truncated values that do not fit in the buffer.

Quoted 6 lines

Quoted 2 lines
Reject truncated values early to prevent boot_kstrtoul() from parsing a partial value and thus from silently using the wrong RSDP address.
And?
If it uses the wrong address, it'll crash'n'burn later. As it should be.

The problem is that we don't necessarily use the user-supplied address.

get_cmdline_acpi_rsdp() can truncate it into a different, parseable address and use that instead. That might not crash at all.

We already return 0 and fail gracefully when boot_kstrtoul() cannot parse the value; this does the same when cmdline_find_option() reports the value was truncated because it didn't fit the buffer.

Borislav Petkov

maintainer

2026-06-18 16:39 UTC #3 lore raw

Quoted 2 lines · Thorsten Blum

get_cmdline_acpi_rsdp() can truncate it into a different, parseable address and use that instead.

How?

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Thorsten Blum

author

2026-06-18 17:59 UTC #4 lore raw

Quoted 5 lines · Borislav Petkov

Quoted 2 lines · Thorsten Blum
get_cmdline_acpi_rsdp() can truncate it into a different, parseable address and use that instead.
How?

The buffer has 19 bytes to hold the "0x" prefix, 16 hex digits, and the NUL terminator.

cmdline_find_option() copies only bufsize - 1 bytes, but returns the full argument length. So for example:

	acpi_rsdp=0x0123456789abcdefx

gets copied as:

	0x0123456789abcdef

which boot_kstrtoul() parses successfully. The user supplied an invalid value, but we silently use the truncated prefix as the RSDP address.

Borislav Petkov

maintainer

2026-06-18 18:04 UTC #5 lore raw

Quoted 13 lines · Thorsten Blum

Quoted 5 lines · Borislav Petkov
Quoted 2 lines · Thorsten Blum
get_cmdline_acpi_rsdp() can truncate it into a different, parseable address and use that instead.
How?
The buffer has 19 bytes to hold the "0x" prefix, 16 hex digits, and the NUL terminator.
cmdline_find_option() copies only bufsize - 1 bytes, but returns the full argument length. So for example:

Quoted 8 lines

	acpi_rsdp=0x0123456789abcdefx
gets copied as:
	0x0123456789abcdef
which boot_kstrtoul() parses successfully. The user supplied an invalid value, but we silently use the truncated prefix as the RSDP address.

My question stands:

"Or are we protecting people from shooting themselves in foot now too?"

Especially users who should know what they're doing...

IOW, how far are we going to "protect" here?

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Thorsten Blum

author

2026-06-18 18:58 UTC #6 lore raw

Quoted 23 lines · Borislav Petkov

Quoted 13 lines · Thorsten Blum
Quoted 5 lines · Borislav Petkov
Quoted 2 lines · Thorsten Blum
get_cmdline_acpi_rsdp() can truncate it into a different, parseable address and use that instead.
How?
The buffer has 19 bytes to hold the "0x" prefix, 16 hex digits, and the NUL terminator.
cmdline_find_option() copies only bufsize - 1 bytes, but returns the full argument length. So for example:
Quoted 8 lines
	acpi_rsdp=0x0123456789abcdefx
gets copied as:
	0x0123456789abcdef
which boot_kstrtoul() parses successfully. The user supplied an invalid value, but we silently use the truncated prefix as the RSDP address.

Quoted 7 lines

My question stands:
"Or are we protecting people from shooting themselves in foot now too?"
Especially users who should know what they're doing...
IOW, how far are we going to "protect" here?

Only far enough to avoid using a value the user didn't actually enter.

A user can still shoot themselves in the foot by using a syntactically valid but wrong address. The check only rejects an overlong acpi_rsdp= value after cmdline_find_option() reports that it didn't fit in the buffer.

We already reject values that boot_kstrtoul() fails to parse; this is the same idea: the copied string is incomplete and shouldn't be parsed.

Thanks,
Thorsten

Borislav Petkov

maintainer

2026-06-18 19:34 UTC #7 lore raw

Quoted 1 line · Thorsten Blum

Only far enough to avoid using a value the user didn't actually enter.

But the user did enter it.

And nothing in there tells her/him that they entered a wrong value. Only that the value she entered magically turned into a 0.

So how is that helping said user fix the input?

And is there a real use case you're fixing here or is this something hypothetical that *might* happen?

Quoted 4 lines

A user can still shoot themselves in the foot by using a syntactically valid but wrong address. The check only rejects an overlong acpi_rsdp= value after cmdline_find_option() reports that it didn't fit in the buffer.

My question still stands paraphrazed: what *actual*, real use case are you fixing here?

And how does your fix make anything better?

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Thorsten Blum

author

2026-06-19 01:01 UTC #8 lore raw

Quoted 18 lines · Borislav Petkov

Quoted 1 line · Thorsten Blum
Only far enough to avoid using a value the user didn't actually enter.
But the user did enter it.
And nothing in there tells her/him that they entered a wrong value. Only that the value she entered magically turned into a 0.
So how is that helping said user fix the input?
And is there a real use case you're fixing here or is this something hypothetical that *might* happen?
Quoted 4 lines
A user can still shoot themselves in the foot by using a syntactically valid but wrong address. The check only rejects an overlong acpi_rsdp= value after cmdline_find_option() reports that it didn't fit in the buffer.

Quoted 4 lines

My question still stands paraphrazed: what *actual*, real use case are you fixing here?
And how does your fix make anything better?

Without this patch, the decompressor acts on a truncated address. The result is undefined and crashed my setup during early boot (an internal broken script surfaced this).

With this patch, a user at least has a chance to see the main kernel's log message:

	[    0.000000] Malformed early option 'acpi_rsdp'

You can reproduce this with QEMU using the malformed example from before:

	acpi_rsdp=0x0123456789abcdefx

Borislav Petkov

maintainer

2026-06-19 02:48 UTC #9 lore raw

Quoted 4 lines · Thorsten Blum

You can reproduce this with QEMU using the malformed example from before:
	acpi_rsdp=0x0123456789abcdefx

I just did: it says

[    0.000000] Malformed early option 'acpi_rsdp'

with latest Linus tree without your patch.

That's because that comes from setup_acpi_rsdp() which calls kstrtoul().

I doubt you even hit get_cmdline_acpi_rsdp() as that's the decompressor legacy path and modern machines boot through the EFI stub like my guest does...

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Thorsten Blum

author

2026-06-19 07:58 UTC #10 lore raw

Quoted 8 lines · Borislav Petkov

Quoted 4 lines · Thorsten Blum
You can reproduce this with QEMU using the malformed example from before:
	acpi_rsdp=0x0123456789abcdefx
I just did: it says

Quoted 8 lines

[    0.000000] Malformed early option 'acpi_rsdp'
with latest Linus tree without your patch.
That's because that comes from setup_acpi_rsdp() which calls kstrtoul().
I doubt you even hit get_cmdline_acpi_rsdp() as that's the decompressor legacy path and modern machines boot through the EFI stub like my guest does...

Are you perhaps appending nokaslr?

With the latest Linus tree, defconfig, and CONFIG_MEMORY_HOTREMOVE=y, this crashes reproducibly for me, but only when KASLR is not disabled:

qemu-system-x86_64 -nographic -no-reboot -kernel arch/x86/boot/bzImage -append "console=ttyS0 acpi_rsdp=0x0123456789abcdefx"

Borislav Petkov

maintainer

2026-06-19 20:24 UTC #11 lore raw

Quoted 1 line · Thorsten Blum

Are you perhaps appending nokaslr?

Yes, removed it. Same thing.

Quoted 4 lines

With the latest Linus tree, defconfig, and CONFIG_MEMORY_HOTREMOVE=y, this crashes reproducibly for me, but only when KASLR is not disabled:
qemu-system-x86_64 -nographic -no-reboot -kernel arch/x86/boot/bzImage -append "console=ttyS0 acpi_rsdp=0x0123456789abcdefx"

As said, efistub entry point bypassing get_cmdline_acpi_rsdp():

...
-drive if=pflash,format=raw,unit=0,file=/home/boris/kvm/debian/uefi/OVMF_CODE_4M-sid-uefi.fd,readonly=on -drive if=pflash,format=raw,unit=1,file=/home/boris/kvm/debian/uefi/OVMF_VARS_4M-sid-uefi.fd,readonly=of
...

So I can't reproduce it.

So, in order not to waste any more time with this silly thing: if we're going to do something here, then that something better warn users that rsdp_addr parsing didn't go as planned. But not silently turn it to 0. It does that now but since you care so much, then fix it right pls.

Thx.

Signature · 4 lines

Regards/Gruss,
    Boris.

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

Thorsten Blum

author

2026-06-19 21:43 UTC #12 lore raw

Quoted 17 lines · Borislav Petkov

Quoted 1 line · Thorsten Blum
Are you perhaps appending nokaslr?
Yes, removed it. Same thing.
Quoted 4 lines
With the latest Linus tree, defconfig, and CONFIG_MEMORY_HOTREMOVE=y, this crashes reproducibly for me, but only when KASLR is not disabled:
qemu-system-x86_64 -nographic -no-reboot -kernel arch/x86/boot/bzImage -append "console=ttyS0 acpi_rsdp=0x0123456789abcdefx"
As said, efistub entry point bypassing get_cmdline_acpi_rsdp():
...
(quoted diff) +0 -2
-drive if=pflash,format=raw,unit=0,file=/home/boris/kvm/debian/uefi/OVMF_CODE_4M-sid-uefi.fd,readonly=on
-drive if=pflash,format=raw,unit=1,file=/home/boris/kvm/debian/uefi/OVMF_VARS_4M-sid-uefi.fd,readonly=of
...

Quoted 1 line

So I can't reproduce it.

Right, but that is a different setup, which bypasses get_cmdline_acpi_rsdp() and is not expected to reproduce this bug.

Could you please try the exact direct kernel boot reproducer I provided, using defconfig and CONFIG_MEMORY_HOTREMOVE=y?

$ qemu-system-x86_64 -nographic -no-reboot -kernel arch/x86/boot/bzImage -append "console=ttyS0 acpi_rsdp=0x0123456789abcdefx"

That path reaches get_cmdline_acpi_rsdp() and should crash during early boot without the patch. Thanks.