Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] arm64: kaslr: Print warning if KASLR is disabled due to lack of seed
@ 2019-11-07 12:12 Mark Brown
  2019-11-08 13:38 ` Catalin Marinas
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2019-11-07 12:12 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon; +Cc: Mark Brown, linux-arm-kernel

It seems to be a relatively common system integration error for system
integrators and administrators to enable KASLR in their configuration but
not provide the seed at runtime, sometimes due to that breaking at some
later point after it is initially enabled. Since KASLR is not announced at
boot time unless it forces on KPTI this can lead to users incorrectly
believing their system has the feature enabled when in fact it does not,
and if they notice the problem the lack of any diagnostics makes it harder
to understand the problem. Provide a warning message to assist in these
situations.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/kaslr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 416f537bf614..c2ba5e783ada 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -98,8 +98,10 @@ u64 __init kaslr_early_init(u64 dt_phys)
 	 * Retrieve (and wipe) the seed from the FDT
 	 */
 	seed = get_kaslr_seed(fdt);
-	if (!seed)
+	if (!seed) {
+		pr_warn("No seed available for KASLR, disabling\n");
 		return 0;
+	}
 
 	/*
 	 * Check if 'nokaslr' appears on the command line, and
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: kaslr: Print warning if KASLR is disabled due to lack of seed
  2019-11-07 12:12 [PATCH] arm64: kaslr: Print warning if KASLR is disabled due to lack of seed Mark Brown
@ 2019-11-08 13:38 ` Catalin Marinas
  2019-11-08 14:14   ` Mark Rutland
  2019-11-08 14:17   ` Mark Brown
  0 siblings, 2 replies; 5+ messages in thread
From: Catalin Marinas @ 2019-11-08 13:38 UTC (permalink / raw)
  To: Mark Brown; +Cc: Will Deacon, linux-arm-kernel

On Thu, Nov 07, 2019 at 12:12:41PM +0000, Mark Brown wrote:
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 416f537bf614..c2ba5e783ada 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -98,8 +98,10 @@ u64 __init kaslr_early_init(u64 dt_phys)
>  	 * Retrieve (and wipe) the seed from the FDT
>  	 */
>  	seed = get_kaslr_seed(fdt);
> -	if (!seed)
> +	if (!seed) {
> +		pr_warn("No seed available for KASLR, disabling\n");
>  		return 0;
> +	}
>  
>  	/*
>  	 * Check if 'nokaslr' appears on the command line, and

For some reason, this patch locks up the VM on TX2, stuck in a recursive
fault. Too early for a printk?

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: kaslr: Print warning if KASLR is disabled due to lack of seed
  2019-11-08 13:38 ` Catalin Marinas
@ 2019-11-08 14:14   ` Mark Rutland
  2019-11-08 14:35     ` Mark Brown
  2019-11-08 14:17   ` Mark Brown
  1 sibling, 1 reply; 5+ messages in thread
From: Mark Rutland @ 2019-11-08 14:14 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Mark Brown, Will Deacon, linux-arm-kernel

On Fri, Nov 08, 2019 at 01:38:31PM +0000, Catalin Marinas wrote:
> On Thu, Nov 07, 2019 at 12:12:41PM +0000, Mark Brown wrote:
> > diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> > index 416f537bf614..c2ba5e783ada 100644
> > --- a/arch/arm64/kernel/kaslr.c
> > +++ b/arch/arm64/kernel/kaslr.c
> > @@ -98,8 +98,10 @@ u64 __init kaslr_early_init(u64 dt_phys)
> >  	 * Retrieve (and wipe) the seed from the FDT
> >  	 */
> >  	seed = get_kaslr_seed(fdt);
> > -	if (!seed)
> > +	if (!seed) {
> > +		pr_warn("No seed available for KASLR, disabling\n");
> >  		return 0;
> > +	}
> >  
> >  	/*
> >  	 * Check if 'nokaslr' appears on the command line, and
> 
> For some reason, this patch locks up the VM on TX2, stuck in a recursive
> fault. Too early for a printk?

We call kaslr_early_init() before start_kernel(), so we haven't set up
things like the per-cpu offset here (and I believe that printk relies on
that internally).

To avoid surprises, I think it'd be best to log that state later, in
setup.c. We can also do that consistently, so that the user has a
positive message when KASLR is in use.

We can either have something like:

void announce_kaslr(void)
{
	if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE))
		return;
	
	if (kaslr_offset() != 0)
		pr_info("KASLR in use\n");
	else if (in_commandline("nokaslr"))
		pr_info("KASLR disabled (command line)\n");
	else
		pr_info("KASLR disabled (no seed)\n");
}

Or have  kaslr.c update something like:

enum kaslr_status {
	KASLR_ENABLED,
	KASLR_DISABLED,
	KASLR_NO_SEED,
};

enum kaslr_status __ro_after_init kaslr_status;

... and switch on that in setup.c.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: kaslr: Print warning if KASLR is disabled due to lack of seed
  2019-11-08 13:38 ` Catalin Marinas
  2019-11-08 14:14   ` Mark Rutland
@ 2019-11-08 14:17   ` Mark Brown
  1 sibling, 0 replies; 5+ messages in thread
From: Mark Brown @ 2019-11-08 14:17 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Will Deacon, linux-arm-kernel

[-- Attachment #1.1: Type: text/plain, Size: 667 bytes --]

On Fri, Nov 08, 2019 at 01:38:31PM +0000, Catalin Marinas wrote:
> On Thu, Nov 07, 2019 at 12:12:41PM +0000, Mark Brown wrote:

> >  	seed = get_kaslr_seed(fdt);
> > -	if (!seed)
> > +	if (!seed) {
> > +		pr_warn("No seed available for KASLR, disabling\n");
> >  		return 0;
> > +	}

> For some reason, this patch locks up the VM on TX2, stuck in a recursive
> fault. Too early for a printk?

I guess so - I was able to trigger this print without problems on my
system locally, not sure what configuration or system changes would make
the difference on TX2 but clearly there's at least some with issues.
That's annoying, I'll add an initcall to do the print instead.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: kaslr: Print warning if KASLR is disabled due to lack of seed
  2019-11-08 14:14   ` Mark Rutland
@ 2019-11-08 14:35     ` Mark Brown
  0 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2019-11-08 14:35 UTC (permalink / raw)
  To: Mark Rutland; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel

[-- Attachment #1.1: Type: text/plain, Size: 913 bytes --]

On Fri, Nov 08, 2019 at 02:14:27PM +0000, Mark Rutland wrote:

> To avoid surprises, I think it'd be best to log that state later, in

I was just going to add an initcall in kaslr.c to keep things together.

> setup.c. We can also do that consistently, so that the user has a
> positive message when KASLR is in use.

So long as people are happy announcing it, I didn't add anything since I
wasn't clear if this was a deliberate decision to not provide
information but I see now it was more likely just the fact that it's
running too early to reliably print.

> enum kaslr_status {
> 	KASLR_ENABLED,
> 	KASLR_DISABLED,
> 	KASLR_NO_SEED,
> };

> enum kaslr_status __ro_after_init kaslr_status;

> ... and switch on that in setup.c.

Having the enum seems like it spreads the code out for unclear
advantage, apart from the logging nothing particularly cares about
anything other than the enabled/disabled decision.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 12:12 [PATCH] arm64: kaslr: Print warning if KASLR is disabled due to lack of seed Mark Brown
2019-11-08 13:38 ` Catalin Marinas
2019-11-08 14:14   ` Mark Rutland
2019-11-08 14:35     ` Mark Brown
2019-11-08 14:17   ` Mark Brown

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git