Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hi Phil,

Thanks for testing this. Can you let me know if v1 of this works?

https://lore.kernel.org/lkml/20220602212234.344394-1-Jason@zx2c4.com/

(I'll also fashion a revert for this part of stable.)

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Phil Elwell]

Hi Jason,

On 07/06/2022 09:30, Jason A. Donenfeld wrote:
> Hi Phil,
> 
> Thanks for testing this. Can you let me know if v1 of this works?
> 
> https://lore.kernel.org/lkml/20220602212234.344394-1-Jason@zx2c4.com/
> 
> (I'll also fashion a revert for this part of stable.)
> 
> Jason

Thanks for the quick response, but that doesn't work for me either. Let me say 
again that I'm on a downstream kernel (rpi-5.15.y) so this may not be a 
universal problem, but merging either of these fixing patches would be fatal for us.

Phil

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hi Phil,

On Tue, Jun 7, 2022 at 10:47 AM Phil Elwell <phil@raspberrypi.com> wrote:
> Thanks for the quick response, but that doesn't work for me either. Let me say
> again that I'm on a downstream kernel (rpi-5.15.y) so this may not be a
> universal problem, but merging either of these fixing patches would be fatal for us.

Alright, thanks. And I'm guessing you don't currently have a problem
*without* either of the fixing patches, because your device tree
doesn't use rng-seed. Is that right?

In anycase, I sent in a revert to get all the static branch stuff out
of stable -- https://lore.kernel.org/stable/20220607084005.666059-1-Jason@zx2c4.com/
-- so the "urgency" of this should decrease and we can fix this as
normal during the 5.19 cycle. But with that said, I do want to get
this fixed as soon as possible still. I'll be back at my desk tonight
and will finally have access to real hardware again. Are you hitting
this by loading a 32bit kernel on a raspi4? Or running a 32bit kernel
on the raspi1? Or some other combo?

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Catalin Marinas]

Hi Jason,

On Tue, Jun 07, 2022 at 10:51:41AM +0200, Jason A. Donenfeld wrote:
> On Tue, Jun 7, 2022 at 10:47 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > Thanks for the quick response, but that doesn't work for me either. Let me say
> > again that I'm on a downstream kernel (rpi-5.15.y) so this may not be a
> > universal problem, but merging either of these fixing patches would be fatal for us.
> 
> Alright, thanks. And I'm guessing you don't currently have a problem
> *without* either of the fixing patches, because your device tree
> doesn't use rng-seed. Is that right?
> 
> In anycase, I sent in a revert to get all the static branch stuff out
> of stable -- https://lore.kernel.org/stable/20220607084005.666059-1-Jason@zx2c4.com/
> -- so the "urgency" of this should decrease and we can fix this as
> normal during the 5.19 cycle.

Since the above revert got queued in -stable, I assume you don't need
commit 73e2d827a501 ("arm64: Initialize jump labels before
setup_machine_fdt()") in stable either.

Do you plan to fix the crng_ready() static branch differently? If you
do, I'd like to revert the corresponding arm64 commit as well. It seems
to be harmless but I'd rather not keep it if no longer needed. So please
keep me updated whatever you decide.

Thanks.

-- 
Catalin

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hi Catalin,

On Tue, Jun 07, 2022 at 11:06:56AM +0100, Catalin Marinas wrote:
> Hi Jason,
> 
> On Tue, Jun 07, 2022 at 10:51:41AM +0200, Jason A. Donenfeld wrote:
> > On Tue, Jun 7, 2022 at 10:47 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > Thanks for the quick response, but that doesn't work for me either. Let me say
> > > again that I'm on a downstream kernel (rpi-5.15.y) so this may not be a
> > > universal problem, but merging either of these fixing patches would be fatal for us.
> > 
> > Alright, thanks. And I'm guessing you don't currently have a problem
> > *without* either of the fixing patches, because your device tree
> > doesn't use rng-seed. Is that right?
> > 
> > In anycase, I sent in a revert to get all the static branch stuff out
> > of stable -- https://lore.kernel.org/stable/20220607084005.666059-1-Jason@zx2c4.com/
> > -- so the "urgency" of this should decrease and we can fix this as
> > normal during the 5.19 cycle.
> 
> Since the above revert got queued in -stable, I assume you don't need
> commit 73e2d827a501 ("arm64: Initialize jump labels before
> setup_machine_fdt()") in stable either.

I made the point here:
https://lore.kernel.org/stable/Yp8i9DH57dRGfTNf@kroah.com/T/#m8f33bc14b677980abe690e5c7a4909b5902010cc

> Do you plan to fix the crng_ready() static branch differently? If you
> do, I'd like to revert the corresponding arm64 commit as well. It seems
> to be harmless but I'd rather not keep it if no longer needed. So please
> keep me updated whatever you decide.

I sent a "backup commit" for that here: https://lore.kernel.org/all/20220607100210.683136-1-Jason@zx2c4.com/
But I would like a few days to see if there's some trivial way of not
needing that on arm32. If it turns out to be easy, then I'd prefer the
direct fix akin to the arm64 one. If it turns out to be not easy, then
I'll merge the backup commit. I'll keep you posted (and I assume anyway
you'll see the arm32 attempts progress or fail here, also).

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hi Catalin,

On Tue, Jun 7, 2022 at 12:07 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> Do you plan to fix the crng_ready() static branch differently? If you
> do, I'd like to revert the corresponding arm64 commit as well. It seems
> to be harmless but I'd rather not keep it if no longer needed. So please
> keep me updated whatever you decide.

Feel free to revert. I'm aborting this line of inquiry and will just
fix it downstream in random.c.

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Catalin Marinas]

Hi Jason,

On Wed, Jun 08, 2022 at 10:20:21AM +0200, Jason A. Donenfeld wrote:
> On Tue, Jun 7, 2022 at 12:07 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > Do you plan to fix the crng_ready() static branch differently? If you
> > do, I'd like to revert the corresponding arm64 commit as well. It seems
> > to be harmless but I'd rather not keep it if no longer needed. So please
> > keep me updated whatever you decide.
> 
> Feel free to revert. I'm aborting this line of inquiry and will just
> fix it downstream in random.c.

Thanks for the update.

-- 
Catalin

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Greg KH]

On Tue, Jun 07, 2022 at 09:47:30AM +0100, Phil Elwell wrote:
> Hi Jason,
> 
> On 07/06/2022 09:30, Jason A. Donenfeld wrote:
> > Hi Phil,
> > 
> > Thanks for testing this. Can you let me know if v1 of this works?
> > 
> > https://lore.kernel.org/lkml/20220602212234.344394-1-Jason@zx2c4.com/
> > 
> > (I'll also fashion a revert for this part of stable.)
> > 
> > Jason
> 
> Thanks for the quick response, but that doesn't work for me either. Let me
> say again that I'm on a downstream kernel (rpi-5.15.y) so this may not be a
> universal problem, but merging either of these fixing patches would be fatal
> for us.

I have reports of a "clean" 5.15.45 working just fine on a rpi.
Anything special in your tree that isn't upstream yet that might be
conflicting with this?  Any chance you can try a kernel.org release
instead?

thanks,

greg k-h

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Phil Elwell]

Hi Greg,

On 07/06/2022 10:10, Greg KH wrote:
> On Tue, Jun 07, 2022 at 09:47:30AM +0100, Phil Elwell wrote:
>> Hi Jason,
>>
>> On 07/06/2022 09:30, Jason A. Donenfeld wrote:
>>> Hi Phil,
>>>
>>> Thanks for testing this. Can you let me know if v1 of this works?
>>>
>>> https://lore.kernel.org/lkml/20220602212234.344394-1-Jason@zx2c4.com/
>>>
>>> (I'll also fashion a revert for this part of stable.)
>>>
>>> Jason
>>
>> Thanks for the quick response, but that doesn't work for me either. Let me
>> say again that I'm on a downstream kernel (rpi-5.15.y) so this may not be a
>> universal problem, but merging either of these fixing patches would be fatal
>> for us.
> 
> I have reports of a "clean" 5.15.45 working just fine on a rpi.
> Anything special in your tree that isn't upstream yet that might be
> conflicting with this?  Any chance you can try a kernel.org release
> instead?

A clean 5.15.45 boots cleanly, whereas a downstream kernel shows the static key 
warning (but it does go on to boot). The significant difference is that our 
defconfigs set CONFIG_RANDOM_TRUST_BOOTLOADER=y - defining that on top of 
multi_v7_defconfig demonstrates the issue on a clean 5.15.45. Conversely, not 
setting that option in a downstream kernel build avoids the warning, presumably 
because it takes much longer to accumulate the required entropy.

Phil

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hi Phil,

On Tue, Jun 07, 2022 at 12:04:13PM +0100, Phil Elwell wrote:
> A clean 5.15.45 boots cleanly, whereas a downstream kernel shows the static key 
> warning (but it does go on to boot). The significant difference is that our 
> defconfigs set CONFIG_RANDOM_TRUST_BOOTLOADER=y - defining that on top of 
> multi_v7_defconfig demonstrates the issue on a clean 5.15.45. Conversely, not 
> setting that option in a downstream kernel build avoids the warning

Ah, that makes sense. Note that I've got a patch out for changing that
defconfig as well to be Y, which means the CI will catch this sort of
thing in the future.

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Russell King (Oracle)]

On Tue, Jun 07, 2022 at 10:30:49AM +0200, Jason A. Donenfeld wrote:
> Hi Phil,
> 
> Thanks for testing this. Can you let me know if v1 of this works?
> 
> https://lore.kernel.org/lkml/20220602212234.344394-1-Jason@zx2c4.com/
> 
> (I'll also fashion a revert for this part of stable.)

As the arm32 version hasn't been merged yet, how is it in stable already?
If it is in stable, isn't that a yet another violation of the stable
kernel rules?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

On 6/7/22, Russell King (Oracle) <linux@armlinux.org.uk> wrote:
> On Tue, Jun 07, 2022 at 10:30:49AM +0200, Jason A. Donenfeld wrote:
>> Hi Phil,
>>
>> Thanks for testing this. Can you let me know if v1 of this works?
>>
>> https://lore.kernel.org/lkml/20220602212234.344394-1-Jason@zx2c4.com/
>>
>> (I'll also fashion a revert for this part of stable.)
>
> As the arm32 version hasn't been merged yet, how is it in stable already?
> If it is in stable, isn't that a yet another violation of the stable
> kernel rules?

You misunderstood my ambiguous sentence, sorry. I meant a revert of
the thing in stable that makes this discussion here a stable
discussion rather than a 5.19 discussion. No arm32 stuff has been
committed anywhere.

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hi Phil,

On Tue, Jun 7, 2022 at 10:29 AM Phil Elwell <phil@raspberrypi.com> wrote:
>
> This patch is fatal for me in the downstream Raspberry Pi kernel - it locks up
> on boot even before the earlycon output is available. Hacking jump_label_init to
> skip the jump_entry for "crng_is_ready" allows it to boot, but is likely to have
> consequences further down the line.

Also, reading this a few times, I'm not 100% sure I understand what
you did to hack around this and why that works. Think you could paste
your hackpatch just out of interest to the discussion (but obviously
not to be applied)?

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Phil Elwell]

On 07/06/2022 09:43, Jason A. Donenfeld wrote:
> Hi Phil,
> 
> On Tue, Jun 7, 2022 at 10:29 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>
>> This patch is fatal for me in the downstream Raspberry Pi kernel - it locks up
>> on boot even before the earlycon output is available. Hacking jump_label_init to
>> skip the jump_entry for "crng_is_ready" allows it to boot, but is likely to have
>> consequences further down the line.
> 
> Also, reading this a few times, I'm not 100% sure I understand what
> you did to hack around this and why that works. Think you could paste
> your hackpatch just out of interest to the discussion (but obviously
> not to be applied)?

This is the minimal version of my workaround patch that at least allows the 
board to boot. Bear in mind that it was written with no previous knowledge of 
jump labels and was arrived at by iteratively bisecting the list of jump_labels 
until the first dangerous one was found, then later working out that there was 
only one.

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b156e152d6b48..7b053521f7216 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -466,6 +466,7 @@ void __init jump_label_init(void)
         struct jump_entry *iter_stop = __stop___jump_table;
         struct static_key *key = NULL;
         struct jump_entry *iter;
+       int iter_count = 0;

         /*
          * Since we are initializing the static_key.enabled field with
@@ -499,7 +500,9 @@ void __init jump_label_init(void)
                         continue;

                 key = iterk;
-               static_key_set_entries(key, iter);
+               iter_count++;
+               if (iter_count != 1083)
+                       static_key_set_entries(key, iter);
         }
         static_key_initialized = true;
         jump_label_unlock();

I'm sure this could be rewritten in a less fragile manner making reference to 
crng_is_ready directly, but this is where I got to yesterday.

Ha! I just proved the patch's fragility by switching from a Pi 2 to a Pi 4,
so the saner version is:

diff --git a/drivers/char/random.c b/drivers/char/random.c
index ca17a658c2147..ca7c8a67b8314 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -79,7 +79,7 @@ static enum {
         CRNG_EARLY = 1, /* At least POOL_EARLY_BITS collected */
         CRNG_READY = 2  /* Fully initialized with POOL_READY_BITS collected */
  } crng_init __read_mostly = CRNG_EMPTY;
-static DEFINE_STATIC_KEY_FALSE(crng_is_ready);
+DEFINE_STATIC_KEY_FALSE(crng_is_ready);
  #define crng_ready() (static_branch_likely(&crng_is_ready) || crng_init >= 
CRNG_READY)
  /* Various types of waiters for crng_init->CRNG_READY transition. */
  static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait);


diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index b156e152d6b48..b79de9da036fd 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -484,6 +484,7 @@ void __init jump_label_init(void)
         jump_label_sort_entries(iter_start, iter_stop);

         for (iter = iter_start; iter < iter_stop; iter++) {
+               extern struct static_key_false crng_is_ready;
                 struct static_key *iterk;
                 bool in_init;

@@ -499,7 +500,8 @@ void __init jump_label_init(void)
                         continue;

                 key = iterk;
-               static_key_set_entries(key, iter);
+               if (key != &crng_is_ready.key)
+                       static_key_set_entries(key, iter);
         }
         static_key_initialized = true;
         jump_label_unlock();

And to answer your questions from the other thread:

* Without any fixing patches we see the warning messages because we are using 
rng-seed in DT.

* I've seen the problem on a Pi 2 and a Pi 4 running 32-bit kernels.

Phil

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hey again,

On Tue, Jun 07, 2022 at 10:15:27AM +0100, Phil Elwell wrote:
> On 07/06/2022 09:43, Jason A. Donenfeld wrote:
> > Hi Phil,
> > 
> > On Tue, Jun 7, 2022 at 10:29 AM Phil Elwell <phil@raspberrypi.com> wrote:
> >>
> >> This patch is fatal for me in the downstream Raspberry Pi kernel - it locks up
> >> on boot even before the earlycon output is available. Hacking jump_label_init to
> >> skip the jump_entry for "crng_is_ready" allows it to boot, but is likely to have
> >> consequences further down the line.
> > 
> > Also, reading this a few times, I'm not 100% sure I understand what
> > you did to hack around this and why that works. Think you could paste
> > your hackpatch just out of interest to the discussion (but obviously
> > not to be applied)?
> 
> This is the minimal version of my workaround patch that at least allows the 
> board to boot. Bear in mind that it was written with no previous knowledge of 
> jump labels and was arrived at by iteratively bisecting the list of jump_labels 
> until the first dangerous one was found, then later working out that there was 
> only one.

Looks like this patch fails due to CONFIG_STRICT_KERNEL_RWX.
Investigating deeper now, but that for starters seems to be the
differentiating factor between my prior test rig and one that reproduces
the error. I assume your raspi also sets CONFIG_STRICT_KERNEL_RWX.

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Phil Elwell]

Jason,

On 07/06/2022 16:14, Jason A. Donenfeld wrote:
> Hey again,
> 
> On Tue, Jun 07, 2022 at 10:15:27AM +0100, Phil Elwell wrote:
>> On 07/06/2022 09:43, Jason A. Donenfeld wrote:
>>> Hi Phil,
>>>
>>> On Tue, Jun 7, 2022 at 10:29 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>>>
>>>> This patch is fatal for me in the downstream Raspberry Pi kernel - it locks up
>>>> on boot even before the earlycon output is available. Hacking jump_label_init to
>>>> skip the jump_entry for "crng_is_ready" allows it to boot, but is likely to have
>>>> consequences further down the line.
>>>
>>> Also, reading this a few times, I'm not 100% sure I understand what
>>> you did to hack around this and why that works. Think you could paste
>>> your hackpatch just out of interest to the discussion (but obviously
>>> not to be applied)?
>>
>> This is the minimal version of my workaround patch that at least allows the
>> board to boot. Bear in mind that it was written with no previous knowledge of
>> jump labels and was arrived at by iteratively bisecting the list of jump_labels
>> until the first dangerous one was found, then later working out that there was
>> only one.
> 
> Looks like this patch fails due to CONFIG_STRICT_KERNEL_RWX.
> Investigating deeper now, but that for starters seems to be the
> differentiating factor between my prior test rig and one that reproduces
> the error. I assume your raspi also sets CONFIG_STRICT_KERNEL_RWX.

Yes, it does, as does multi_v7_defconfig.

Phil

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Hi Phil,

On Tue, Jun 07, 2022 at 04:35:32PM +0100, Phil Elwell wrote:
> Jason,
> 
> On 07/06/2022 16:14, Jason A. Donenfeld wrote:
> > Hey again,
> > 
> > On Tue, Jun 07, 2022 at 10:15:27AM +0100, Phil Elwell wrote:
> >> On 07/06/2022 09:43, Jason A. Donenfeld wrote:
> >>> Hi Phil,
> >>>
> >>> On Tue, Jun 7, 2022 at 10:29 AM Phil Elwell <phil@raspberrypi.com> wrote:
> >>>>
> >>>> This patch is fatal for me in the downstream Raspberry Pi kernel - it locks up
> >>>> on boot even before the earlycon output is available. Hacking jump_label_init to
> >>>> skip the jump_entry for "crng_is_ready" allows it to boot, but is likely to have
> >>>> consequences further down the line.
> >>>
> >>> Also, reading this a few times, I'm not 100% sure I understand what
> >>> you did to hack around this and why that works. Think you could paste
> >>> your hackpatch just out of interest to the discussion (but obviously
> >>> not to be applied)?
> >>
> >> This is the minimal version of my workaround patch that at least allows the
> >> board to boot. Bear in mind that it was written with no previous knowledge of
> >> jump labels and was arrived at by iteratively bisecting the list of jump_labels
> >> until the first dangerous one was found, then later working out that there was
> >> only one.
> > 
> > Looks like this patch fails due to CONFIG_STRICT_KERNEL_RWX.
> > Investigating deeper now, but that for starters seems to be the
> > differentiating factor between my prior test rig and one that reproduces
> > the error. I assume your raspi also sets CONFIG_STRICT_KERNEL_RWX.
> 
> Yes, it does, as does multi_v7_defconfig.

Oh good. Adjusting my CI now to have that.

Having tickled arch/arm/ a little bit now, this is looking sort of
complicated. So I think I might be leaning toward giving up and just
rolling with <https://git.zx2c4.com/linux-rng/commit/?id=78f79dda>.

Unless of course somebody has some ARM chops and can think of a quick
easy fix.

Jason

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

Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

[Phil Elwell]

On 07/06/2022 16:44, Jason A. Donenfeld wrote:
> Hi Phil,
> 
> On Tue, Jun 07, 2022 at 04:35:32PM +0100, Phil Elwell wrote:
>> Jason,
>>
>> On 07/06/2022 16:14, Jason A. Donenfeld wrote:
>>> Hey again,
>>>
>>> On Tue, Jun 07, 2022 at 10:15:27AM +0100, Phil Elwell wrote:
>>>> On 07/06/2022 09:43, Jason A. Donenfeld wrote:
>>>>> Hi Phil,
>>>>>
>>>>> On Tue, Jun 7, 2022 at 10:29 AM Phil Elwell <phil@raspberrypi.com> wrote:
>>>>>>
>>>>>> This patch is fatal for me in the downstream Raspberry Pi kernel - it locks up
>>>>>> on boot even before the earlycon output is available. Hacking jump_label_init to
>>>>>> skip the jump_entry for "crng_is_ready" allows it to boot, but is likely to have
>>>>>> consequences further down the line.
>>>>>
>>>>> Also, reading this a few times, I'm not 100% sure I understand what
>>>>> you did to hack around this and why that works. Think you could paste
>>>>> your hackpatch just out of interest to the discussion (but obviously
>>>>> not to be applied)?
>>>>
>>>> This is the minimal version of my workaround patch that at least allows the
>>>> board to boot. Bear in mind that it was written with no previous knowledge of
>>>> jump labels and was arrived at by iteratively bisecting the list of jump_labels
>>>> until the first dangerous one was found, then later working out that there was
>>>> only one.
>>>
>>> Looks like this patch fails due to CONFIG_STRICT_KERNEL_RWX.
>>> Investigating deeper now, but that for starters seems to be the
>>> differentiating factor between my prior test rig and one that reproduces
>>> the error. I assume your raspi also sets CONFIG_STRICT_KERNEL_RWX.
>>
>> Yes, it does, as does multi_v7_defconfig.
> 
> Oh good. Adjusting my CI now to have that.
> 
> Having tickled arch/arm/ a little bit now, this is looking sort of
> complicated. So I think I might be leaning toward giving up and just
> rolling with <https://git.zx2c4.com/linux-rng/commit/?id=78f79dda>.
> 
> Unless of course somebody has some ARM chops and can think of a quick
> easy fix.

I've not looked at the performance implications (if any), but in terms of when the RNG initilialization completes and the lack of a WARN I'm happy with that patch, a.k.a. [1].

Phil

[1] https://lore.kernel.org/lkml/20220607100210.683136-1-Jason@zx2c4.com/

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

[PATCH v3] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

Stephen reported that a static key warning splat appears during early
boot on arm64 systems that credit randomness from device trees that
contain an "rng-seed" property, because setup_machine_fdt() is called
before jump_label_init() during setup_arch(), which was fixed by
73e2d827a501 ("arm64: Initialize jump labels before
setup_machine_fdt()").

The same basic issue applies to arm32 as well. So this commit adds a
call to jump_label_init() just before setup_machine_fdt(). Since the
page maps haven't been set yet, this also requires us to use the early
patching code in the jump label code.

Reported-by: Stephen Boyd <swboyd@chromium.org>
Reported-by: Phil Elwell <phil@raspberrypi.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Russel King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Fixes: f5bda35fba61 ("random: use static branch for crng_ready()")
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/arm/kernel/jump_label.c | 3 ++-
 arch/arm/kernel/setup.c      | 1 +
 arch/arm/mm/mmu.c            | 3 +++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/jump_label.c b/arch/arm/kernel/jump_label.c
index 303b3ab87f7e..8f8c5312f917 100644
--- a/arch/arm/kernel/jump_label.c
+++ b/arch/arm/kernel/jump_label.c
@@ -8,6 +8,7 @@ static void __arch_jump_label_transform(struct jump_entry *entry,
 					enum jump_label_type type,
 					bool is_static)
 {
+	extern bool early_mm_initialized;
 	void *addr = (void *)entry->code;
 	unsigned int insn;
 
@@ -16,7 +17,7 @@ static void __arch_jump_label_transform(struct jump_entry *entry,
 	else
 		insn = arm_gen_nop();
 
-	if (is_static)
+	if (is_static || !early_mm_initialized)
 		__patch_text_early(addr, insn);
 	else
 		patch_text(addr, insn);
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1e8a50a97edf..3ff80b1ee0b5 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1101,6 +1101,7 @@ void __init setup_arch(char **cmdline_p)
 		atags_vaddr = FDT_VIRT_BASE(__atags_pointer);
 
 	setup_processor();
+	jump_label_init();
 	if (atags_vaddr) {
 		mdesc = setup_machine_fdt(atags_vaddr);
 		if (mdesc)
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 5e2be37a198e..3f63a5581966 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1754,10 +1754,13 @@ void __init paging_init(const struct machine_desc *mdesc)
 	__flush_dcache_page(NULL, empty_zero_page);
 }
 
+bool early_mm_initialized;
+
 void __init early_mm_init(const struct machine_desc *mdesc)
 {
 	build_mem_type_table();
 	early_paging_init(mdesc);
+	early_mm_initialized = true;
 }
 
 void set_pte_at(struct mm_struct *mm, unsigned long addr,
-- 
2.35.1


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

Re: [PATCH v3] ARM: initialize jump labels before setup_machine_fdt()

[Jason A. Donenfeld]

This patch isn't needed in the end. An equivalent patch is needed on
xtensa, powerpc, arc, mips, arm32, arm64, riscv. That's a bit much and
points to a larger issue. So I'll fix this the ugly way in the
random.c code :(.

Jason

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