Re: [PATCH] kaslr: x86: fixes log message nokaslr

[Mahmoud Younes]

Thanks a lot for the reply. here is my thought process:


On Sun, 31 Mar 2024 at 22:28, Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> On Sun, Mar 31, 2024 at 10:05:46PM +0200, Mahmoud Younes wrote:
> > Unknown kernel command line parameters nokaslr message will be printed
> > to kernel log buffer if nokaslr option exists in boot command line.
> > nokaslr gets consumed earlier and this message becomes confusing.
> > impact is that user gets confused whether kaslr is enabled or not.
>
> Well, my dmesg has here:
>
> ---
> ...
> trampoline_32bit: 0x0000000000000000
>
>
> KASLR disabled: 'nokaslr' on cmdline.
>
>
> Decompressing Linux... Parsing ELF... No relocation needed... done.
> ...
> ---
>
> so the notification for the user is there.
>

I don't think this gets printed after executing dmesg. This gets
printed to console if earlyprintks are captured to the console and the
kernel is configured with printing low level debug info. that is not
the default behavior and requires manipulating the kernel
configuration and command line to get this message. Specifically,
enabling DEBUG_LL and EARLY_PRINTK and adding "earlyprintk=ttyS0" or
whatever console in use. the message is just not visible in dmesg even
though it would exist in kernel log buffer, just the console wouldn't
be initialized at that moment. moreover, this is not visible when
booting a real hardware.

> > Signed-off-by: Mahmoud Younes <m.younesbadr@xxxxxxxxx>
> > ---
> >  arch/x86/mm/kaslr.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> > index 37db264866b6..a62cb0675e22 100644
> > --- a/arch/x86/mm/kaslr.c
> > +++ b/arch/x86/mm/kaslr.c
> > @@ -179,3 +179,9 @@ void __meminit init_trampoline_kaslr(void)
> >                       __pgd(_KERNPG_TABLE | __pa(pud_page_tramp));
> >       }
> >  }
> > +
> > +static int __init parse_nokaslr(char *_)
> > +{
> > +     return 0;
> > +}
> > +early_param("nokaslr", parse_nokaslr);
>
> This piece of code without any comments explaining why it is there is
> not less confusing to whoever stares at it.
>
> I'd prefer if print_unknown_bootoptions() would filter out those options
> which are parsed earlier and not warn about them instead of having such
> dummy stubs.
>
as far as I understand, nokaslr is not treated as any type of param.
IOW semantically it is an early param, but it will not be defined
during runtime in any of the sections specified for kernel parameters.
I am thinking of three ways to handle this issue of filtering
parameters that are parsed earlier. First way, include them in a
section (.init.setup?, __param?, new_section? the first being more
logical). Thinking about it, nokaslr is semantically an early_param
(consumed early in the boot process -- probably way too early). Hence,
it's logical to add definition for a corresponding obs_kernel_param
object for nokaslr to .init.setup via the early_param macro. The issue
is, by the time all params in this section are handled, kaslr init
code will have been executed and nokaslr won't have an effect. The not
so elegant way to handle this is the empty stub submitted and here as
well https://lore.kernel.org/linux-arm-kernel/20230412043258.397455-1-quic_pkondeti@xxxxxxxxxxx/T/.
The elegant way is to define nokaslr as early_param, write a handler
that sets a boolean flag whether to enable or disable kaslr, call
parse_early_param before setup_arch (I believe this is not possible
due to the dependency; the code in early param handlers is dependent
on setup_arch being executed first) and replace the code that checks
for nokaslr in the command line with just checking the flag.
second way, provide a static array of params known to be consumed
during early boot but are not known to the kernel which does not sound
elegant to me.
Third way, leave everything as is and ignore the issue.

Since I am new to the kernel code base, I would appreciate some
guidance on how to move forward. Thank you!

Best,
M. Younes