linux-kernel - Re: [PATCH v2] ARM: initialize jump labels before setup_machine

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8c3fe744-0181-043a-3af9-dd00165a6356@raspberrypi.com>
Date:   Tue, 7 Jun 2022 10:15:27 +0100
From:   Phil Elwell <phil@...pberrypi.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     Russell King - ARM Linux <linux@...linux.org.uk>,
        Russell King <rmk+kernel@...linux.org.uk>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Stephen Boyd <swboyd@...omium.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH v2] ARM: initialize jump labels before setup_machine_fdt()

On 07/06/2022 09:43, Jason A. Donenfeld wrote:
> Hi Phil,
> 
> On Tue, Jun 7, 2022 at 10:29 AM Phil Elwell <phil@...pberrypi.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