* [PATCH] arm64: kaslr: Use standard early random function
@ 2020-08-07 0:49 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-08-07 0:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, linux-kernel,
Daniel Díaz, tytso, Guenter Roeck, Qian Cai, Mark Brown
Commit 585524081ecd ("random: random.h should include archrandom.h, not
the other way around") tries to fix a problem with recursive inclusion
of linux/random.h and arch/archrandom.h for arm64. Unfortunately, this
results in the following compile error if ARCH_RANDOM is disabled.
arch/arm64/kernel/kaslr.c: In function 'kaslr_early_init':
arch/arm64/kernel/kaslr.c:128:6: error: implicit declaration of function
'__early_cpu_has_rndr'; did you mean '__early_pfn_to_nid'?
[-Werror=implicit-function-declaration]
if (__early_cpu_has_rndr()) {
^~~~~~~~~~~~~~~~~~~~
__early_pfn_to_nid
arch/arm64/kernel/kaslr.c:131:7: error: implicit declaration of function
'__arm64_rndr' [-Werror=implicit-function-declaration]
if (__arm64_rndr(&raw))
^~~~~~~~~~~~
Problem is that arch/archrandom.h is only included from linux/random.h if
ARCH_RANDOM is enabled. If not, __arm64_rndr() and __early_cpu_has_rndr()
are undeclared, causing the problem.
Use arch_get_random_seed_long_early() instead of arm64 specific functions
to solve the problem. As a side effect of this change, the code no longer
bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
was disabled for a reason).
Reported-by: Qian Cai <cai@lca.pw>
Fixes: 585524081ecd ("random: random.h should include archrandom.h, not the other way around")
Fixes: 2e8e1ea88cbc ("arm64: Use v8.5-RNG entropy for KASLR seed")
Cc: Qian Cai <cai@lca.pw>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
arch/arm64/kernel/kaslr.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 9ded4237e1c1..b181e0544b79 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -84,6 +84,7 @@ u64 __init kaslr_early_init(u64 dt_phys)
void *fdt;
u64 seed, offset, mask, module_range;
const u8 *cmdline, *str;
+ unsigned long raw;
int size;
/*
@@ -122,15 +123,12 @@ u64 __init kaslr_early_init(u64 dt_phys)
}
/*
- * Mix in any entropy obtainable architecturally, open coded
- * since this runs extremely early.
+ * Mix in any entropy obtainable architecturally if enabled
+ * and supported.
*/
- if (__early_cpu_has_rndr()) {
- unsigned long raw;
- if (__arm64_rndr(&raw))
- seed ^= raw;
- }
+ if (arch_get_random_seed_long_early(&raw))
+ seed ^= raw;
if (!seed) {
kaslr_status = KASLR_DISABLED_NO_SEED;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] arm64: kaslr: Use standard early random function
@ 2020-08-07 0:49 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-08-07 0:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Daniel Díaz, tytso, Catalin Marinas, linux-kernel,
Mark Brown, Qian Cai, Guenter Roeck, Will Deacon,
linux-arm-kernel
Commit 585524081ecd ("random: random.h should include archrandom.h, not
the other way around") tries to fix a problem with recursive inclusion
of linux/random.h and arch/archrandom.h for arm64. Unfortunately, this
results in the following compile error if ARCH_RANDOM is disabled.
arch/arm64/kernel/kaslr.c: In function 'kaslr_early_init':
arch/arm64/kernel/kaslr.c:128:6: error: implicit declaration of function
'__early_cpu_has_rndr'; did you mean '__early_pfn_to_nid'?
[-Werror=implicit-function-declaration]
if (__early_cpu_has_rndr()) {
^~~~~~~~~~~~~~~~~~~~
__early_pfn_to_nid
arch/arm64/kernel/kaslr.c:131:7: error: implicit declaration of function
'__arm64_rndr' [-Werror=implicit-function-declaration]
if (__arm64_rndr(&raw))
^~~~~~~~~~~~
Problem is that arch/archrandom.h is only included from linux/random.h if
ARCH_RANDOM is enabled. If not, __arm64_rndr() and __early_cpu_has_rndr()
are undeclared, causing the problem.
Use arch_get_random_seed_long_early() instead of arm64 specific functions
to solve the problem. As a side effect of this change, the code no longer
bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
was disabled for a reason).
Reported-by: Qian Cai <cai@lca.pw>
Fixes: 585524081ecd ("random: random.h should include archrandom.h, not the other way around")
Fixes: 2e8e1ea88cbc ("arm64: Use v8.5-RNG entropy for KASLR seed")
Cc: Qian Cai <cai@lca.pw>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
arch/arm64/kernel/kaslr.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 9ded4237e1c1..b181e0544b79 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -84,6 +84,7 @@ u64 __init kaslr_early_init(u64 dt_phys)
void *fdt;
u64 seed, offset, mask, module_range;
const u8 *cmdline, *str;
+ unsigned long raw;
int size;
/*
@@ -122,15 +123,12 @@ u64 __init kaslr_early_init(u64 dt_phys)
}
/*
- * Mix in any entropy obtainable architecturally, open coded
- * since this runs extremely early.
+ * Mix in any entropy obtainable architecturally if enabled
+ * and supported.
*/
- if (__early_cpu_has_rndr()) {
- unsigned long raw;
- if (__arm64_rndr(&raw))
- seed ^= raw;
- }
+ if (arch_get_random_seed_long_early(&raw))
+ seed ^= raw;
if (!seed) {
kaslr_status = KASLR_DISABLED_NO_SEED;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
2020-08-07 0:49 ` Guenter Roeck
@ 2020-08-07 2:17 ` Linus Torvalds
-1 siblings, 0 replies; 10+ messages in thread
From: Linus Torvalds @ 2020-08-07 2:17 UTC (permalink / raw)
To: Guenter Roeck
Cc: Catalin Marinas, Will Deacon, Linux ARM,
Linux Kernel Mailing List, Daniel Díaz, Theodore Ts'o,
Qian Cai, Mark Brown
On Thu, Aug 6, 2020 at 5:49 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Use arch_get_random_seed_long_early() instead of arm64 specific functions
> to solve the problem. As a side effect of this change, the code no longer
> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
> was disabled for a reason).
This patch looks sane to me, but let's see what the arm64 people say
in case they have preferences..
Linus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
@ 2020-08-07 2:17 ` Linus Torvalds
0 siblings, 0 replies; 10+ messages in thread
From: Linus Torvalds @ 2020-08-07 2:17 UTC (permalink / raw)
To: Guenter Roeck
Cc: Daniel Díaz, Theodore Ts'o, Catalin Marinas,
Linux Kernel Mailing List, Mark Brown, Qian Cai, Will Deacon,
Linux ARM
On Thu, Aug 6, 2020 at 5:49 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Use arch_get_random_seed_long_early() instead of arm64 specific functions
> to solve the problem. As a side effect of this change, the code no longer
> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
> was disabled for a reason).
This patch looks sane to me, but let's see what the arm64 people say
in case they have preferences..
Linus
_______________________________________________
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] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
2020-08-07 0:49 ` Guenter Roeck
@ 2020-08-07 10:17 ` Mark Rutland
-1 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2020-08-07 10:17 UTC (permalink / raw)
To: Guenter Roeck
Cc: Linus Torvalds, Catalin Marinas, Will Deacon, linux-arm-kernel,
linux-kernel, Daniel Díaz, tytso, Qian Cai, Mark Brown
Hi Guenter,
On Thu, Aug 06, 2020 at 05:49:04PM -0700, Guenter Roeck wrote:
> Commit 585524081ecd ("random: random.h should include archrandom.h, not
> the other way around") tries to fix a problem with recursive inclusion
> of linux/random.h and arch/archrandom.h for arm64. Unfortunately, this
> results in the following compile error if ARCH_RANDOM is disabled.
>
> arch/arm64/kernel/kaslr.c: In function 'kaslr_early_init':
> arch/arm64/kernel/kaslr.c:128:6: error: implicit declaration of function
> '__early_cpu_has_rndr'; did you mean '__early_pfn_to_nid'?
> [-Werror=implicit-function-declaration]
> if (__early_cpu_has_rndr()) {
> ^~~~~~~~~~~~~~~~~~~~
> __early_pfn_to_nid
> arch/arm64/kernel/kaslr.c:131:7: error: implicit declaration of function
> '__arm64_rndr' [-Werror=implicit-function-declaration]
> if (__arm64_rndr(&raw))
> ^~~~~~~~~~~~
>
> Problem is that arch/archrandom.h is only included from linux/random.h if
> ARCH_RANDOM is enabled. If not, __arm64_rndr() and __early_cpu_has_rndr()
> are undeclared, causing the problem.
>
> Use arch_get_random_seed_long_early() instead of arm64 specific functions
> to solve the problem. As a side effect of this change, the code no longer
> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
> was disabled for a reason).
There's no bypass of ARCH_RANDOM; the bits KASLR depends on are empty
stubs when ARCH_RANDOM is not selected. I added the common early
functions after this code was written.
> Reported-by: Qian Cai <cai@lca.pw>
> Fixes: 585524081ecd ("random: random.h should include archrandom.h, not the other way around")
This is where things broke; there was no need to change kaslr.c's
include of <asm/archrandom.h>, since kaslr.c only depends on the RNDR
bits defined there./
> Fixes: 2e8e1ea88cbc ("arm64: Use v8.5-RNG entropy for KASLR seed")
I don't think this tag is necessary; this commit built and worked fine,
and there wasn't any ARCH_RANDOM bypass to speak of.
> Cc: Qian Cai <cai@lca.pw>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
This patch itself looks fine, but as above I think the commit message is
misleading w.r.t. bypassing ARCH_RANDOM, and the second fixes tag isn't
necessary.
With those bits gone:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/kernel/kaslr.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 9ded4237e1c1..b181e0544b79 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -84,6 +84,7 @@ u64 __init kaslr_early_init(u64 dt_phys)
> void *fdt;
> u64 seed, offset, mask, module_range;
> const u8 *cmdline, *str;
> + unsigned long raw;
> int size;
>
> /*
> @@ -122,15 +123,12 @@ u64 __init kaslr_early_init(u64 dt_phys)
> }
>
> /*
> - * Mix in any entropy obtainable architecturally, open coded
> - * since this runs extremely early.
> + * Mix in any entropy obtainable architecturally if enabled
> + * and supported.
> */
> - if (__early_cpu_has_rndr()) {
> - unsigned long raw;
>
> - if (__arm64_rndr(&raw))
> - seed ^= raw;
> - }
> + if (arch_get_random_seed_long_early(&raw))
> + seed ^= raw;
>
> if (!seed) {
> kaslr_status = KASLR_DISABLED_NO_SEED;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
@ 2020-08-07 10:17 ` Mark Rutland
0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2020-08-07 10:17 UTC (permalink / raw)
To: Guenter Roeck
Cc: Daniel Díaz, tytso, Will Deacon, Catalin Marinas,
linux-kernel, Mark Brown, Qian Cai, Linus Torvalds,
linux-arm-kernel
Hi Guenter,
On Thu, Aug 06, 2020 at 05:49:04PM -0700, Guenter Roeck wrote:
> Commit 585524081ecd ("random: random.h should include archrandom.h, not
> the other way around") tries to fix a problem with recursive inclusion
> of linux/random.h and arch/archrandom.h for arm64. Unfortunately, this
> results in the following compile error if ARCH_RANDOM is disabled.
>
> arch/arm64/kernel/kaslr.c: In function 'kaslr_early_init':
> arch/arm64/kernel/kaslr.c:128:6: error: implicit declaration of function
> '__early_cpu_has_rndr'; did you mean '__early_pfn_to_nid'?
> [-Werror=implicit-function-declaration]
> if (__early_cpu_has_rndr()) {
> ^~~~~~~~~~~~~~~~~~~~
> __early_pfn_to_nid
> arch/arm64/kernel/kaslr.c:131:7: error: implicit declaration of function
> '__arm64_rndr' [-Werror=implicit-function-declaration]
> if (__arm64_rndr(&raw))
> ^~~~~~~~~~~~
>
> Problem is that arch/archrandom.h is only included from linux/random.h if
> ARCH_RANDOM is enabled. If not, __arm64_rndr() and __early_cpu_has_rndr()
> are undeclared, causing the problem.
>
> Use arch_get_random_seed_long_early() instead of arm64 specific functions
> to solve the problem. As a side effect of this change, the code no longer
> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
> was disabled for a reason).
There's no bypass of ARCH_RANDOM; the bits KASLR depends on are empty
stubs when ARCH_RANDOM is not selected. I added the common early
functions after this code was written.
> Reported-by: Qian Cai <cai@lca.pw>
> Fixes: 585524081ecd ("random: random.h should include archrandom.h, not the other way around")
This is where things broke; there was no need to change kaslr.c's
include of <asm/archrandom.h>, since kaslr.c only depends on the RNDR
bits defined there./
> Fixes: 2e8e1ea88cbc ("arm64: Use v8.5-RNG entropy for KASLR seed")
I don't think this tag is necessary; this commit built and worked fine,
and there wasn't any ARCH_RANDOM bypass to speak of.
> Cc: Qian Cai <cai@lca.pw>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
This patch itself looks fine, but as above I think the commit message is
misleading w.r.t. bypassing ARCH_RANDOM, and the second fixes tag isn't
necessary.
With those bits gone:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/kernel/kaslr.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 9ded4237e1c1..b181e0544b79 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -84,6 +84,7 @@ u64 __init kaslr_early_init(u64 dt_phys)
> void *fdt;
> u64 seed, offset, mask, module_range;
> const u8 *cmdline, *str;
> + unsigned long raw;
> int size;
>
> /*
> @@ -122,15 +123,12 @@ u64 __init kaslr_early_init(u64 dt_phys)
> }
>
> /*
> - * Mix in any entropy obtainable architecturally, open coded
> - * since this runs extremely early.
> + * Mix in any entropy obtainable architecturally if enabled
> + * and supported.
> */
> - if (__early_cpu_has_rndr()) {
> - unsigned long raw;
>
> - if (__arm64_rndr(&raw))
> - seed ^= raw;
> - }
> + if (arch_get_random_seed_long_early(&raw))
> + seed ^= raw;
>
> if (!seed) {
> kaslr_status = KASLR_DISABLED_NO_SEED;
> --
> 2.17.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] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
2020-08-07 0:49 ` Guenter Roeck
@ 2020-08-07 11:09 ` Mark Brown
-1 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2020-08-07 11:09 UTC (permalink / raw)
To: Guenter Roeck
Cc: Linus Torvalds, Catalin Marinas, Will Deacon, linux-arm-kernel,
linux-kernel, Daniel Díaz, tytso, Qian Cai
[-- Attachment #1: Type: text/plain, Size: 598 bytes --]
On Thu, Aug 06, 2020 at 05:49:04PM -0700, Guenter Roeck wrote:
> Use arch_get_random_seed_long_early() instead of arm64 specific functions
> to solve the problem. As a side effect of this change, the code no longer
> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
> was disabled for a reason).
This should be fine since the entire implementation is still static
inline in the header other than the prints in the WARN_ON() which should
never be triggered here so it should be fine.
Reviewed-by: Mark Brown <broonie@kernel.org>
Tested-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
@ 2020-08-07 11:09 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2020-08-07 11:09 UTC (permalink / raw)
To: Guenter Roeck
Cc: Daniel Díaz, tytso, Will Deacon, Catalin Marinas,
linux-kernel, Qian Cai, Linus Torvalds, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 598 bytes --]
On Thu, Aug 06, 2020 at 05:49:04PM -0700, Guenter Roeck wrote:
> Use arch_get_random_seed_long_early() instead of arm64 specific functions
> to solve the problem. As a side effect of this change, the code no longer
> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
> was disabled for a reason).
This should be fine since the entire implementation is still static
inline in the header other than the prints in the WARN_ON() which should
never be triggered here so it should be fine.
Reviewed-by: Mark Brown <broonie@kernel.org>
Tested-by: Mark Brown <broonie@kernel.org>
[-- 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] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
2020-08-07 10:17 ` Mark Rutland
@ 2020-08-07 14:41 ` Guenter Roeck
-1 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-08-07 14:41 UTC (permalink / raw)
To: Mark Rutland
Cc: Linus Torvalds, Catalin Marinas, Will Deacon, linux-arm-kernel,
linux-kernel, Daniel Díaz, tytso, Qian Cai, Mark Brown
On 8/7/20 3:17 AM, Mark Rutland wrote:
> Hi Guenter,
>
> On Thu, Aug 06, 2020 at 05:49:04PM -0700, Guenter Roeck wrote:
>> Commit 585524081ecd ("random: random.h should include archrandom.h, not
>> the other way around") tries to fix a problem with recursive inclusion
>> of linux/random.h and arch/archrandom.h for arm64. Unfortunately, this
>> results in the following compile error if ARCH_RANDOM is disabled.
>>
>> arch/arm64/kernel/kaslr.c: In function 'kaslr_early_init':
>> arch/arm64/kernel/kaslr.c:128:6: error: implicit declaration of function
>> '__early_cpu_has_rndr'; did you mean '__early_pfn_to_nid'?
>> [-Werror=implicit-function-declaration]
>> if (__early_cpu_has_rndr()) {
>> ^~~~~~~~~~~~~~~~~~~~
>> __early_pfn_to_nid
>> arch/arm64/kernel/kaslr.c:131:7: error: implicit declaration of function
>> '__arm64_rndr' [-Werror=implicit-function-declaration]
>> if (__arm64_rndr(&raw))
>> ^~~~~~~~~~~~
>>
>> Problem is that arch/archrandom.h is only included from linux/random.h if
>> ARCH_RANDOM is enabled. If not, __arm64_rndr() and __early_cpu_has_rndr()
>> are undeclared, causing the problem.
>>
>> Use arch_get_random_seed_long_early() instead of arm64 specific functions
>> to solve the problem. As a side effect of this change, the code no longer
>> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
>> was disabled for a reason).
>
> There's no bypass of ARCH_RANDOM; the bits KASLR depends on are empty
> stubs when ARCH_RANDOM is not selected. I added the common early
> functions after this code was written.
>
>> Reported-by: Qian Cai <cai@lca.pw>
>> Fixes: 585524081ecd ("random: random.h should include archrandom.h, not the other way around")
>
> This is where things broke; there was no need to change kaslr.c's
> include of <asm/archrandom.h>, since kaslr.c only depends on the RNDR
> bits defined there./
>
Problem is that, prior to the above patch, asm/archrandom.h used to include
linux/random.h, which resulted in a recursion. That only worked because
some other include file had already included linux/random.h in kaslr.c.
In v5.7.y, with gcc 7.x and older, that include did not happen, causing
a compile failure.
In file included from ./arch/arm64/include/asm/archrandom.h:9:0,
from arch/arm64/kernel/kaslr.c:14:
./include/linux/random.h: In function ‘arch_get_random_seed_long_early’:
./include/linux/random.h:149:9: error: implicit declaration of function ‘arch_get_random_seed_long’
>> Fixes: 2e8e1ea88cbc ("arm64: Use v8.5-RNG entropy for KASLR seed")
>
> I don't think this tag is necessary; this commit built and worked fine,
> and there wasn't any ARCH_RANDOM bypass to speak of.
>
>> Cc: Qian Cai <cai@lca.pw>
>> Cc: Mark Brown <broonie@kernel.org>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> This patch itself looks fine, but as above I think the commit message is
> misleading w.r.t. bypassing ARCH_RANDOM, and the second fixes tag isn't
> necessary.
>
You are correct. I'll change that and resubmit.
Thanks,
Guenter
> With those bits gone:
>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>
> Mark.
>
>> ---
>> arch/arm64/kernel/kaslr.c | 12 +++++-------
>> 1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
>> index 9ded4237e1c1..b181e0544b79 100644
>> --- a/arch/arm64/kernel/kaslr.c
>> +++ b/arch/arm64/kernel/kaslr.c
>> @@ -84,6 +84,7 @@ u64 __init kaslr_early_init(u64 dt_phys)
>> void *fdt;
>> u64 seed, offset, mask, module_range;
>> const u8 *cmdline, *str;
>> + unsigned long raw;
>> int size;
>>
>> /*
>> @@ -122,15 +123,12 @@ u64 __init kaslr_early_init(u64 dt_phys)
>> }
>>
>> /*
>> - * Mix in any entropy obtainable architecturally, open coded
>> - * since this runs extremely early.
>> + * Mix in any entropy obtainable architecturally if enabled
>> + * and supported.
>> */
>> - if (__early_cpu_has_rndr()) {
>> - unsigned long raw;
>>
>> - if (__arm64_rndr(&raw))
>> - seed ^= raw;
>> - }
>> + if (arch_get_random_seed_long_early(&raw))
>> + seed ^= raw;
>>
>> if (!seed) {
>> kaslr_status = KASLR_DISABLED_NO_SEED;
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: kaslr: Use standard early random function
@ 2020-08-07 14:41 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-08-07 14:41 UTC (permalink / raw)
To: Mark Rutland
Cc: Daniel Díaz, tytso, Will Deacon, Catalin Marinas,
linux-kernel, Mark Brown, Qian Cai, Linus Torvalds,
linux-arm-kernel
On 8/7/20 3:17 AM, Mark Rutland wrote:
> Hi Guenter,
>
> On Thu, Aug 06, 2020 at 05:49:04PM -0700, Guenter Roeck wrote:
>> Commit 585524081ecd ("random: random.h should include archrandom.h, not
>> the other way around") tries to fix a problem with recursive inclusion
>> of linux/random.h and arch/archrandom.h for arm64. Unfortunately, this
>> results in the following compile error if ARCH_RANDOM is disabled.
>>
>> arch/arm64/kernel/kaslr.c: In function 'kaslr_early_init':
>> arch/arm64/kernel/kaslr.c:128:6: error: implicit declaration of function
>> '__early_cpu_has_rndr'; did you mean '__early_pfn_to_nid'?
>> [-Werror=implicit-function-declaration]
>> if (__early_cpu_has_rndr()) {
>> ^~~~~~~~~~~~~~~~~~~~
>> __early_pfn_to_nid
>> arch/arm64/kernel/kaslr.c:131:7: error: implicit declaration of function
>> '__arm64_rndr' [-Werror=implicit-function-declaration]
>> if (__arm64_rndr(&raw))
>> ^~~~~~~~~~~~
>>
>> Problem is that arch/archrandom.h is only included from linux/random.h if
>> ARCH_RANDOM is enabled. If not, __arm64_rndr() and __early_cpu_has_rndr()
>> are undeclared, causing the problem.
>>
>> Use arch_get_random_seed_long_early() instead of arm64 specific functions
>> to solve the problem. As a side effect of this change, the code no longer
>> bypasses ARCH_RANDOM, which I consider desirable (after all, ARCH_RANDOM
>> was disabled for a reason).
>
> There's no bypass of ARCH_RANDOM; the bits KASLR depends on are empty
> stubs when ARCH_RANDOM is not selected. I added the common early
> functions after this code was written.
>
>> Reported-by: Qian Cai <cai@lca.pw>
>> Fixes: 585524081ecd ("random: random.h should include archrandom.h, not the other way around")
>
> This is where things broke; there was no need to change kaslr.c's
> include of <asm/archrandom.h>, since kaslr.c only depends on the RNDR
> bits defined there./
>
Problem is that, prior to the above patch, asm/archrandom.h used to include
linux/random.h, which resulted in a recursion. That only worked because
some other include file had already included linux/random.h in kaslr.c.
In v5.7.y, with gcc 7.x and older, that include did not happen, causing
a compile failure.
In file included from ./arch/arm64/include/asm/archrandom.h:9:0,
from arch/arm64/kernel/kaslr.c:14:
./include/linux/random.h: In function ‘arch_get_random_seed_long_early’:
./include/linux/random.h:149:9: error: implicit declaration of function ‘arch_get_random_seed_long’
>> Fixes: 2e8e1ea88cbc ("arm64: Use v8.5-RNG entropy for KASLR seed")
>
> I don't think this tag is necessary; this commit built and worked fine,
> and there wasn't any ARCH_RANDOM bypass to speak of.
>
>> Cc: Qian Cai <cai@lca.pw>
>> Cc: Mark Brown <broonie@kernel.org>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> This patch itself looks fine, but as above I think the commit message is
> misleading w.r.t. bypassing ARCH_RANDOM, and the second fixes tag isn't
> necessary.
>
You are correct. I'll change that and resubmit.
Thanks,
Guenter
> With those bits gone:
>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>
> Mark.
>
>> ---
>> arch/arm64/kernel/kaslr.c | 12 +++++-------
>> 1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
>> index 9ded4237e1c1..b181e0544b79 100644
>> --- a/arch/arm64/kernel/kaslr.c
>> +++ b/arch/arm64/kernel/kaslr.c
>> @@ -84,6 +84,7 @@ u64 __init kaslr_early_init(u64 dt_phys)
>> void *fdt;
>> u64 seed, offset, mask, module_range;
>> const u8 *cmdline, *str;
>> + unsigned long raw;
>> int size;
>>
>> /*
>> @@ -122,15 +123,12 @@ u64 __init kaslr_early_init(u64 dt_phys)
>> }
>>
>> /*
>> - * Mix in any entropy obtainable architecturally, open coded
>> - * since this runs extremely early.
>> + * Mix in any entropy obtainable architecturally if enabled
>> + * and supported.
>> */
>> - if (__early_cpu_has_rndr()) {
>> - unsigned long raw;
>>
>> - if (__arm64_rndr(&raw))
>> - seed ^= raw;
>> - }
>> + if (arch_get_random_seed_long_early(&raw))
>> + seed ^= raw;
>>
>> if (!seed) {
>> kaslr_status = KASLR_DISABLED_NO_SEED;
>> --
>> 2.17.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] 10+ messages in thread
end of thread, other threads:[~2020-08-07 14:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-07 0:49 [PATCH] arm64: kaslr: Use standard early random function Guenter Roeck
2020-08-07 0:49 ` Guenter Roeck
2020-08-07 2:17 ` Linus Torvalds
2020-08-07 2:17 ` Linus Torvalds
2020-08-07 10:17 ` Mark Rutland
2020-08-07 10:17 ` Mark Rutland
2020-08-07 14:41 ` Guenter Roeck
2020-08-07 14:41 ` Guenter Roeck
2020-08-07 11:09 ` Mark Brown
2020-08-07 11:09 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.