[PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Nir Lichtman]

Problem: Currently booting the kernel with "nokaslr" on x86 incorrectly
doesn't recognize this parameter and passes it on to the init process

Reason: On x86, this parameter is parsed by the early loader, and so the
main kernel itself doesn't do anything with it.

Example: I have encountered this issue when booting the kernel with QEMU
using -kernel and -initrd with a simple initrd I have built containing
bash as the init executable. Upon running init, the kernel passed the
"unrecognized parameter" to bash causing bash to exit with a failure and
the kernel to panic.

Solution: Ingest this parameter as part of the kernel logic in x86 so the
kernel will recognize this as a valid parameter and not pass it to user
mode.
This is similar to the logic that already exists in the case of ARM64
which can be found in arch/arm64/kernel/kaslr.c:43

Crediting also Mahmoud since he came up with a similar patch a couple of
months ago, the patch was left with unaddressed CR comments by Borislav
which I have addressed in this patch,
link to thread: https://lore.kernel.org/all/20240331200546.869343-1-m.younesbadr@gmail.com/

v2: Remove single line if-condition braces, following check patch script warning

Signed-off-by: Mahmoud Younes <m.younesbadr@gmail.com>
Signed-off-by: Nir Lichtman <nir@lichtman.org>
---

Note: resending this since I got no reply on my previous email that was sent a couple of months ago, trying from my second email (perhaps my primary email got to spam)

 arch/x86/mm/kaslr.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 230f1dee4f09..e2aaa72be749 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -71,6 +71,20 @@ static inline unsigned long get_padding(struct kaslr_memory_region *region)
 	return (region->size_tb << TB_SHIFT);
 }
 
+/*
+ * nokaslr param handling is done by the loader which treats the
+ * boot parameters as read only so this is a hack to ingest this
+ * to keep it from passing to user mode
+ */
+static int __init parse_nokaslr(char *p)
+{
+	if (!!(boot_params.hdr.loadflags & KASLR_FLAG))
+		pr_warn("the loader has not parsed the nokaslr flag");
+
+	return 0;
+}
+early_param("nokaslr", parse_nokaslr);
+
 /* Initialize base and padding for each memory region randomized with KASLR */
 void __init kernel_randomize_memory(void)
 {
-- 
2.39.2

Re: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Peter Zijlstra]

On Sat, Jan 11, 2025 at 02:17:43PM +0000, Nir Lichtman wrote:
> Problem: Currently booting the kernel with "nokaslr" on x86 incorrectly
> doesn't recognize this parameter and passes it on to the init process

What now? I'm pretty sure nokaslr works, it's in all my test machines
and qemu scripts. Without it faddr2line wouldn't work, which I would
notice in a hurry.

Re: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Borislav Petkov]

On Tue, Jan 14, 2025 at 10:36:43AM +0100, Peter Zijlstra wrote:
> On Sat, Jan 11, 2025 at 02:17:43PM +0000, Nir Lichtman wrote:
> > Problem: Currently booting the kernel with "nokaslr" on x86 incorrectly
> > doesn't recognize this parameter and passes it on to the init process
> 
> What now? I'm pretty sure nokaslr works, it's in all my test machines
> and qemu scripts. Without it faddr2line wouldn't work, which I would
> notice in a hurry.

Yeah, the param itself works - he doesn't want to pass it on to the init
process because then it says:

[    0.073613] Unknown kernel command line parameters "nokaslr", will be passed to user space.

due to our detection of which parameters got eaten and which didn't.

I'm still "meh" on this thing...

Frankly, I'd prefer if print_unknown_bootoptions() actually did it all itself,
without spreading out silly code which is only to shut up a warning...

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Peter Zijlstra]

On Tue, Jan 14, 2025 at 01:31:42PM +0100, Borislav Petkov wrote:
> On Tue, Jan 14, 2025 at 10:36:43AM +0100, Peter Zijlstra wrote:
> > On Sat, Jan 11, 2025 at 02:17:43PM +0000, Nir Lichtman wrote:
> > > Problem: Currently booting the kernel with "nokaslr" on x86 incorrectly
> > > doesn't recognize this parameter and passes it on to the init process
> > 
> > What now? I'm pretty sure nokaslr works, it's in all my test machines
> > and qemu scripts. Without it faddr2line wouldn't work, which I would
> > notice in a hurry.
> 
> Yeah, the param itself works - he doesn't want to pass it on to the init
> process because then it says:
> 
> [    0.073613] Unknown kernel command line parameters "nokaslr", will be passed to user space.
> 
> due to our detection of which parameters got eaten and which didn't.

I had never noticed; random machine of mine says:

[    0.591565] Unknown kernel command line parameters "force_early_printk nokaslr kgdboc=ttyS0", will be passed to user space.

*shrug*, who cares. 

RE: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Nir Lichtman]

> On Tue, Jan 14, 2025 at 01:31:42PM +0100, Borislav Petkov wrote:
> > On Tue, Jan 14, 2025 at 10:36:43AM +0100, Peter Zijlstra wrote:
> > > On Sat, Jan 11, 2025 at 02:17:43PM +0000, Nir Lichtman wrote:
> > > > Problem: Currently booting the kernel with "nokaslr" on x86
> > > > incorrectly doesn't recognize this parameter and passes it on to
> > > > the init process
> > >
> > > What now? I'm pretty sure nokaslr works, it's in all my test
> > > machines and qemu scripts. Without it faddr2line wouldn't work,
> > > which I would notice in a hurry.
> >
> > Yeah, the param itself works - he doesn't want to pass it on to the
> > init process because then it says:
> >
> > [    0.073613] Unknown kernel command line parameters "nokaslr", will be
> passed to user space.
> >
> > due to our detection of which parameters got eaten and which didn't.
> 
> I had never noticed; random machine of mine says:
> 
> [    0.591565] Unknown kernel command line parameters "force_early_printk
> nokaslr kgdboc=ttyS0", will be passed to user space.
> 
> *shrug*, who cares.

I agree that the warning is not a big deal, thing is the kernel has a fallback behavior
in which unrecognized boot parameters are passed to the init process, this causes
the nokaslr to be passed to the init process, you probably haven't stumbled upon
this since it may be swallowed in your system, but when I made an initramfs with bash
as the init process, bash got the nokaslr as an argument and crashed since it treated it
as a file.

Borslav, print_unknown_bootoptions is an interesting alternative idea,
I could amend this patch to swallow the early parameters over there,
Thing is this, from what I understand it would require the code to keep
a list of possible early parameters and check if one of them arrived into the
print_unknown_bootoptions function and if so swallow in that function,
what do you think about this idea, to implement this?

Thanks,
Nir

Re: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Borislav Petkov]

On Tue, Jan 14, 2025 at 02:37:52PM +0000, Nir Lichtman wrote:
> I agree that the warning is not a big deal, thing is the kernel has
> a fallback behavior in which unrecognized boot parameters are passed to the
> init process, this causes the nokaslr to be passed to the init process, you
> probably haven't stumbled upon this since it may be swallowed in your
> system, but when I made an initramfs with bash as the init process, bash got
> the nokaslr as an argument and crashed since it treated it as a file.

Come again?! By that logic bash would be crashing left'n'right since the
kernel has been doing this forever.

> Borslav, print_unknown_bootoptions is an interesting alternative idea,
> I could amend this patch to swallow the early parameters over there,
> Thing is this, from what I understand it would require the code to keep
> a list of possible early parameters and check if one of them arrived into the
> print_unknown_bootoptions function and if so swallow in that function,
> what do you think about this idea, to implement this?

That's exactly why I say it is "meh". Not convinced that adding a bunch of
code just to prevent warnings...

Looking at __setup_param, it does already stick those params into a separate
section. Now, if print_unknown_bootoptions() would be smart enough to inspect
that section, to compare strings and swallow a param on a match, that might be
a relatively clever way of fixing this without doing any explicit hackery ...

I'd say.

-- 
Regards/Gruss,
    Boris.

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

RE: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Nir Lichtman]

> 
> On Tue, Jan 14, 2025 at 02:37:52PM +0000, Nir Lichtman wrote:
> > I agree that the warning is not a big deal, thing is the kernel has a
> > fallback behavior in which unrecognized boot parameters are passed to
> > the init process, this causes the nokaslr to be passed to the init
> > process, you probably haven't stumbled upon this since it may be
> > swallowed in your system, but when I made an initramfs with bash as
> > the init process, bash got the nokaslr as an argument and crashed since it
> treated it as a file.
> 
> Come again?! By that logic bash would be crashing left'n'right since the kernel
> has been doing this forever.

Yes indeed, when I made an initramfs with cpio and put a static bash build as ./init,
It got the nokaslr as the first argument and crashed, this causes a general panic since
the init process crashed.

This behavior of passing unrecognized params to the init process is documented
in the kernel docs, and also in the corresponding warning message.

> 
> > Borslav, print_unknown_bootoptions is an interesting alternative idea,
> > I could amend this patch to swallow the early parameters over there,
> > Thing is this, from what I understand it would require the code to
> > keep a list of possible early parameters and check if one of them
> > arrived into the print_unknown_bootoptions function and if so swallow
> > in that function, what do you think about this idea, to implement this?
> 
> That's exactly why I say it is "meh". Not convinced that adding a bunch of code
> just to prevent warnings...
> 
> Looking at __setup_param, it does already stick those params into a separate
> section. Now, if print_unknown_bootoptions() would be smart enough to
> inspect that section, to compare strings and swallow a param on a match, that
> might be a relatively clever way of fixing this without doing any explicit hackery

Sounds interesting, I'll take a look at that.

> ...
> 
> I'd say.
> 
> --
> Regards/Gruss,
>     Boris.
> 

Re: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Borislav Petkov]

On Tue, Jan 14, 2025 at 03:02:34PM +0000, Nir Lichtman wrote:
> Yes indeed, when I made an initramfs with cpio and put a static bash build
> as ./init, It got the nokaslr as the first argument and crashed, this causes
> a general panic since the init process crashed.
> 
> This behavior of passing unrecognized params to the init process is
> documented in the kernel docs, and also in the corresponding warning
> message.

So you must be doing something special because all of our systems are running
fine with "nokaslr". As said, stuff would be breaking left'n'right if this
were really a problem.

-- 
Regards/Gruss,
    Boris.

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

RE: [PATCH v2 RESEND] x86/kaslr: Ingest nokaslr to avoid passing it to init process

[Nir Lichtman]

 > On Tue, Jan 14, 2025 at 03:02:34PM +0000, Nir Lichtman wrote:
> > Yes indeed, when I made an initramfs with cpio and put a static bash
> > build as ./init, It got the nokaslr as the first argument and crashed,
> > this causes a general panic since the init process crashed.
> >
> > This behavior of passing unrecognized params to the init process is
> > documented in the kernel docs, and also in the corresponding warning
> > message.
> 
> So you must be doing something special because all of our systems are
> running fine with "nokaslr". As said, stuff would be breaking left'n'right if this
> were really a problem.

To make sure tested it again and it indeed causes a panic in this scenario,
Here are the commands I used for my setup (on Debian x86_64):

apt download bash-static
dpkg -x bash-static_5.2.15-2+b7_amd64.deb .
mv bin/bash-static init
echo init | cpio -H newc -o > init.cpio
make defconfig
make -j 4
qemu-system-x86_64 -kernel arch/x86/boot/bzImage \
-initrd ~/fun/init.cpio -append nokaslr

The "nokaslr" gets passed into bash which causes it to exit, which in turn
causes Linux to panic. Works fine when not passing "nokaslr".

Anyway, I agree that this is a special setup and hence the problem is
not critical, at the time I originally sent this I thought it was more of a
big deal, but now I just tend to work with more minimal kernel configs and in
this case just have KASLR disabled. But still I personally think it would be
more clean to have this fixed, when I have some more time I'll take a look
at what you suggested around the unknown boot parameters handling.

Thanks,
Nir

> 
> --
> Regards/Gruss,
>     Boris.