All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Stephen Boyd <swboyd@chromium.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Russell King <linux@armlinux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>, Phil Elwell <phil@raspberrypi.com>
Subject: Re: [PATCH] random: do not use jump labels before they are initialized
Date: Tue, 7 Jun 2022 13:10:52 +0200	[thread overview]
Message-ID: <CAMj1kXFK9pFUdOABKP0Zp7tEJNVS1dTjxp5DgSwqzM8TEYJLTQ@mail.gmail.com> (raw)
In-Reply-To: <Yp8wz2Ey4J4u+ZlK@zx2c4.com>

On Tue, 7 Jun 2022 at 13:04, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Ard,
>
> On Tue, Jun 07, 2022 at 12:56:20PM +0200, Ard Biesheuvel wrote:
> > Could we do this to defer the static key manipulation? That way, the
> > first call to crng_reseed() that occurs after the static keys API
> > becomes available will set the static key, and patch itself away at
> > the same time.
>
> That's almost the same as the patch I just posted, except you
> pushed the logic down into crng_reseed() instead of credit_init_bits().

Sure.

> (A previous mini-project aimed to remove as much logic as possible from
> crng_reseed(), counting on those blocks in crng_init_bits() to only ever
> run once.) What this means is that the static key won't get changed
> until whenever the next reseeding is. I guess that's "fine" but I think
> I'd prefer to keep the entropy counting stuff as separate from the init
> bits stuff as possible.
>

Fair enough. What I would like is to remove the need to play around
with the placement of jump_label_init() across architectures. Jump
labels are fundamentally a performance optimization, so unless you can
explain how setting it as early as possible makes a material
difference, performance or otherwise, I really think we should pursue
a solution that does the static key manipulation at some later time.

> >> As a third, I could just defer doing anything with the bootloader seed
> >> until random_init(). This might actually be the simplest solution...
> >> I'll sketch something out. A downside, which might be sort of
> >> significant, is that a few odd things actually use randomness before
> >> random_init() is called. So these would miss out on having that seed.
> >> I'll have to look what exactly to see if we're actually getting anything
> >> real out of that.
> >>
> >
> > This is kind of the point of using a firmware provided seed, i.e.,
> > that it is available much earlier than anything else.
>
> I'll send a patch for this anyway because I'm sort of curious now. Maybe
> it'll be a dead end, for the reason you mentioned, but I think I'll
> still try to evaluate it.
>

Sure. Anything that can be deferred to an initcall() should be, as the
early arch code is much too fragile to much around with.

WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ardb@kernel.org>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	 Stephen Boyd <swboyd@chromium.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	 Russell King <linux@armlinux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>,
	 Phil Elwell <phil@raspberrypi.com>
Subject: Re: [PATCH] random: do not use jump labels before they are initialized
Date: Tue, 7 Jun 2022 13:10:52 +0200	[thread overview]
Message-ID: <CAMj1kXFK9pFUdOABKP0Zp7tEJNVS1dTjxp5DgSwqzM8TEYJLTQ@mail.gmail.com> (raw)
In-Reply-To: <Yp8wz2Ey4J4u+ZlK@zx2c4.com>

On Tue, 7 Jun 2022 at 13:04, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Ard,
>
> On Tue, Jun 07, 2022 at 12:56:20PM +0200, Ard Biesheuvel wrote:
> > Could we do this to defer the static key manipulation? That way, the
> > first call to crng_reseed() that occurs after the static keys API
> > becomes available will set the static key, and patch itself away at
> > the same time.
>
> That's almost the same as the patch I just posted, except you
> pushed the logic down into crng_reseed() instead of credit_init_bits().

Sure.

> (A previous mini-project aimed to remove as much logic as possible from
> crng_reseed(), counting on those blocks in crng_init_bits() to only ever
> run once.) What this means is that the static key won't get changed
> until whenever the next reseeding is. I guess that's "fine" but I think
> I'd prefer to keep the entropy counting stuff as separate from the init
> bits stuff as possible.
>

Fair enough. What I would like is to remove the need to play around
with the placement of jump_label_init() across architectures. Jump
labels are fundamentally a performance optimization, so unless you can
explain how setting it as early as possible makes a material
difference, performance or otherwise, I really think we should pursue
a solution that does the static key manipulation at some later time.

> >> As a third, I could just defer doing anything with the bootloader seed
> >> until random_init(). This might actually be the simplest solution...
> >> I'll sketch something out. A downside, which might be sort of
> >> significant, is that a few odd things actually use randomness before
> >> random_init() is called. So these would miss out on having that seed.
> >> I'll have to look what exactly to see if we're actually getting anything
> >> real out of that.
> >>
> >
> > This is kind of the point of using a firmware provided seed, i.e.,
> > that it is available much earlier than anything else.
>
> I'll send a patch for this anyway because I'm sort of curious now. Maybe
> it'll be a dead end, for the reason you mentioned, but I think I'll
> still try to evaluate it.
>

Sure. Anything that can be deferred to an initcall() should be, as the
early arch code is much too fragile to much around with.

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

  reply	other threads:[~2022-06-07 11:12 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-07 10:02 [PATCH] random: do not use jump labels before they are initialized Jason A. Donenfeld
2022-06-07 10:02 ` Jason A. Donenfeld
2022-06-07 10:13 ` Ard Biesheuvel
2022-06-07 10:13   ` Ard Biesheuvel
2022-06-07 10:28   ` Jason A. Donenfeld
2022-06-07 10:28     ` Jason A. Donenfeld
2022-06-07 10:41     ` Jason A. Donenfeld
2022-06-07 10:41       ` Jason A. Donenfeld
2022-06-07 10:56       ` Ard Biesheuvel
2022-06-07 10:56         ` Ard Biesheuvel
2022-06-07 11:04         ` Jason A. Donenfeld
2022-06-07 11:04           ` Jason A. Donenfeld
2022-06-07 11:10           ` Ard Biesheuvel [this message]
2022-06-07 11:10             ` Ard Biesheuvel
2022-06-07 11:35             ` Jason A. Donenfeld
2022-06-07 11:35               ` Jason A. Donenfeld
2022-06-07 12:03               ` Ard Biesheuvel
2022-06-07 12:03                 ` Ard Biesheuvel
2022-06-07 12:16                 ` Jason A. Donenfeld
2022-06-07 12:16                   ` Jason A. Donenfeld
2022-06-07 12:21                   ` Ard Biesheuvel
2022-06-07 12:21                     ` Ard Biesheuvel
2022-06-08  8:24 ` Ard Biesheuvel
2022-06-08  8:24   ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMj1kXFK9pFUdOABKP0Zp7tEJNVS1dTjxp5DgSwqzM8TEYJLTQ@mail.gmail.com \
    --to=ardb@kernel.org \
    --cc=Jason@zx2c4.com \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=phil@raspberrypi.com \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.