On Wed, Aug 5, 2020 at 11:24 AM Guenter Roeck <[email protected]> wrote:
>
> Same with older versions of gcc. I don't see the problem with the
> mainline kernel.
https://www.youtube.com/watch?v=-b5aW08ivHU
> I think this is caused by more recursive includes.
> arch/arm64/include/asm/archrandom.h includes include/linux/random.h
> which includes arch/arm64/include/asm/archrandom.h to get the definition
> of arch_get_random_seed_long_early (which it won't get because of
> the recursion).
>
> What I don't really understand is how this works with new versions
> of gcc.
Is that the only place it triggers?
Because the trivial fix would be something like the appended, which is
the right thing to do anyway.
Linus
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 07c4c8cc4a67..9ded4237e1c1 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -11,8 +11,8 @@
#include <linux/sched.h>
#include <linux/types.h>
#include <linux/pgtable.h>
+#include <linux/random.h>
-#include <asm/archrandom.h>
#include <asm/cacheflush.h>
#include <asm/fixmap.h>
#include <asm/kernel-pgtable.h>
On 8/5/20 11:37 AM, Linus Torvalds wrote:
> On Wed, Aug 5, 2020 at 11:24 AM Guenter Roeck <[email protected]> wrote:
>>
>> Same with older versions of gcc. I don't see the problem with the
>> mainline kernel.
>
> https://www.youtube.com/watch?v=-b5aW08ivHU
>
>> I think this is caused by more recursive includes.
>> arch/arm64/include/asm/archrandom.h includes include/linux/random.h
>> which includes arch/arm64/include/asm/archrandom.h to get the definition
>> of arch_get_random_seed_long_early (which it won't get because of
>> the recursion).
>>
>> What I don't really understand is how this works with new versions
>> of gcc.
>
> Is that the only place it triggers?
>
> Because the trivial fix would be something like the appended, which is
> the right thing to do anyway.
>
Correct.
gcc-7.x and older don't support CONFIG_ARM64_PTR_AUTH. Result is that
./arch/arm64/include/asm/pointer_auth.h doesn't include <linux/random.h>
for those compiler versions, which results in the problem.
In the mainline kernel, ./arch/arm64/include/asm/pointer_auth.h
always includes <linux/random.h>, so the problem isn't seen (or, rather,
it is hidden) there.
The problem is caused (exposed) by Marc's commit ("arm64: Workaround
circular dependency in pointer_auth.h"), which makes the include of
linux/random.h conditional.
Guenter
> Linus
>
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 07c4c8cc4a67..9ded4237e1c1 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -11,8 +11,8 @@
> #include <linux/sched.h>
> #include <linux/types.h>
> #include <linux/pgtable.h>
> +#include <linux/random.h>
>
> -#include <asm/archrandom.h>
> #include <asm/cacheflush.h>
> #include <asm/fixmap.h>
> #include <asm/kernel-pgtable.h>
>
Hello!
On Wed, 5 Aug 2020 at 13:37, Linus Torvalds
<[email protected]> wrote:
> On Wed, Aug 5, 2020 at 11:24 AM Guenter Roeck <[email protected]> wrote:
> >
> > Same with older versions of gcc. I don't see the problem with the
> > mainline kernel.
>
> https://www.youtube.com/watch?v=-b5aW08ivHU
>
> > I think this is caused by more recursive includes.
> > arch/arm64/include/asm/archrandom.h includes include/linux/random.h
> > which includes arch/arm64/include/asm/archrandom.h to get the definition
> > of arch_get_random_seed_long_early (which it won't get because of
> > the recursion).
> >
> > What I don't really understand is how this works with new versions
> > of gcc.
>
> Is that the only place it triggers?
Yes
> Because the trivial fix would be something like the appended, which is
> the right thing to do anyway.
>
> Linus
>
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 07c4c8cc4a67..9ded4237e1c1 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -11,8 +11,8 @@
> #include <linux/sched.h>
> #include <linux/types.h>
> #include <linux/pgtable.h>
> +#include <linux/random.h>
>
> -#include <asm/archrandom.h>
> #include <asm/cacheflush.h>
> #include <asm/fixmap.h>
> #include <asm/kernel-pgtable.h>
Tested-by: Daniel Díaz <[email protected]>
This patch works fine. Built with gcc 7.3.0 and glibc 2.27.
Greetings!
Daniel Díaz
[email protected]
On Wed, Aug 5, 2020 at 12:24 PM Guenter Roeck <[email protected]> wrote:
>
> On 8/5/20 11:37 AM, Linus Torvalds wrote:
> >
> > Because the trivial fix would be something like the appended, which is
> > the right thing to do anyway.
>
> Correct.
I'll take that as an Ack, and also remove the crazy reverse include
from archrandom.h that most definitely shouldn't be there.
It's now commit 585524081ecd ("random: random.h should include
archrandom.h, not the other way around") in my tree, because a grep
for "archrandom.h" shows that now the only place it exists is
<linux/random.h> and a few files that cannot possibly affect arm64
(because they are on x86 and powerpc, neither of which has that insane
reverse include).
Linus
On Wed, Aug 05, 2020 at 12:45:25PM -0700, Linus Torvalds wrote:
> On Wed, Aug 5, 2020 at 12:24 PM Guenter Roeck <[email protected]> wrote:
> >
> > On 8/5/20 11:37 AM, Linus Torvalds wrote:
> > >
> > > Because the trivial fix would be something like the appended, which is
> > > the right thing to do anyway.
> >
> > Correct.
>
> I'll take that as an Ack, and also remove the crazy reverse include
> from archrandom.h that most definitely shouldn't be there.
>
> It's now commit 585524081ecd ("random: random.h should include
> archrandom.h, not the other way around") in my tree, because a grep
> for "archrandom.h" shows that now the only place it exists is
> <linux/random.h> and a few files that cannot possibly affect arm64
> (because they are on x86 and powerpc, neither of which has that insane
> reverse include).
Thanks, I've queued this up for 5.7.y now. Doesn't look relevant for
older kernels, but I haven't gotten reports of them not building just
yet :)
thanks,
greg k-h
On 8/5/20 12:45 PM, Linus Torvalds wrote:
> On Wed, Aug 5, 2020 at 12:24 PM Guenter Roeck <[email protected]> wrote:
>>
>> On 8/5/20 11:37 AM, Linus Torvalds wrote:
>>>
>>> Because the trivial fix would be something like the appended, which is
>>> the right thing to do anyway.
>>
>> Correct.
>
> I'll take that as an Ack, and also remove the crazy reverse include
> from archrandom.h that most definitely shouldn't be there.
>
Thanks, appreciated.
> It's now commit 585524081ecd ("random: random.h should include
> archrandom.h, not the other way around") in my tree, because a grep
> for "archrandom.h" shows that now the only place it exists is
> <linux/random.h> and a few files that cannot possibly affect arm64
> (because they are on x86 and powerpc, neither of which has that insane
> reverse include).
>
I tried to build arm64 on current mainline, after reverting the dts
patches which cause the build failures there. Builds fine with both
gcc 7.4.0 and 9.3.0. 5.7.14-rc2 builds as well with both old and new
compilers. So hopefully we are fine.
Guenter