linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	x86@kernel.org, Nadia Heninger <nadiah@cs.ucsd.edu>,
	Thomas Ristenpart <ristenpart@cornell.edu>,
	Theodore Ts'o <tytso@mit.edu>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>,
	Florian Weimer <fweimer@redhat.com>
Subject: Re: [PATCH RFC v1] random: implement getrandom() in vDSO
Date: Tue, 2 Aug 2022 01:16:56 +0200	[thread overview]
Message-ID: <Yuhe6IIFXqNMZs5b@zx2c4.com> (raw)
In-Reply-To: <87zggnsqwj.ffs@tglx>

On Mon, Aug 01, 2022 at 09:30:20PM +0200, Thomas Gleixner wrote:
> Jason!

Thomas!

> > So, anyway, if I do muster a v2 of this (perhaps just to see the idea
> > through), the API might split in two to something like:
> >
> >   void *getrandom_allocate_states([inout] size_t *number_of_states, [out] size_t *length_per_state);
> >   ssize_t getrandom(void *state, void *buffer, size_t len, unsigned long flags);
> 
> I'm not seeing any reason to have those functions at all.
> 
> The only thing which would be VDSO worthy here is the access to
> random_state->ready and random_state->generation as that's the
> information which is otherwise not available to userspace.

I think you might have missed the part of the patch message where I
discuss this. I'm happy to talk about that more, but it might help the
discussion to refer to the parts already addressed. Reproduced here:

| How do we rectify this? By putting a safe implementation of getrandom()
| in the vDSO, which has access to whatever information a
| particular iteration of random.c is using to make its decisions. I use
| that careful language of "particular iteration of random.c", because the
| set of things that a vDSO getrandom() implementation might need for making
| decisions as good as the kernel's will likely change over time. This
| isn't just a matter of exporting certain *data* to userspace. We're not
| going to commit to a "data API" where the various heuristics used are
| exposed, locking in how the kernel works for decades to come, and then
| leave it to various userspaces to roll something on top and shoot
| themselves in the foot and have all sorts of complexity disasters.
| Rather, vDSO getrandom() is supposed to be the *same exact algorithm*
| that runs in the kernel, except it's been hoisted into userspace as
| much as possible. And so vDSO getrandom() and kernel getrandom() will
| always mirror each other hermetically.

To reiterate, I don't want to commit to a particular data API, or even
to an ideal interplay between kernel random and user random. I'd like to
retain the latitude to change the semantics there considerably, so that
Linux isn't locked into one RNG design forever. I think that kind of
lock in would be a mistake. For example, just the generation counter
alone won't do it (as I mentioned later on in the message; the RFC patch
is somewhat incomplete). Rather, the interface I'm fine committing to
would be the higher level getrandom(), with maybe an added state
parameter, which doesn't expose any guts about what it's actually doing.

Comex (CC'd) described in a forum comment the idea (and perhaps vDSO in
general?) as a little more akin to system libraries on Windows or macOS,
which represent the OS barrier, rather than the raw system call. Such
libraries then can operate on private data as necessary. So in that
sense, this patch here isn't very Linuxy (which Comex described as a
potentially positive thing, but I assume you disagree).

Anyway, I guess it in large part isn't so dissimilar to decisions you
made around other vDSO functions, where to draw the barrier, etc. Why
not just have an accessor for each vvar struct member and leave it to
userspaces to implement? Well, that'd probably be a terrible idea for
various reasons, and I feel the same way about exposing too many
getrandom() guts.

> So you can just have:
> 
>    int random_check_and_update_generation(u64 *generation);
> 
> Everything else is library material, really.

Not very appealing for the reasons mentioned above, but also for the
record, I may like this idea for a closely related thing, vmgenid, but
that's a different conversation I'll get back to another time.

Jason

  reply	other threads:[~2022-08-01 23:17 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-29 14:55 [PATCH RFC v1] random: implement getrandom() in vDSO Jason A. Donenfeld
2022-07-29 20:19 ` Florian Weimer
2022-07-29 22:06   ` Jason A. Donenfeld
2022-07-30 15:48 ` Linus Torvalds
2022-07-30 23:45   ` Jason A. Donenfeld
2022-07-31  0:23     ` Jason A. Donenfeld
2022-07-31  1:31       ` [PATCH RFC v2] " Jason A. Donenfeld
2022-08-01  8:48         ` Florian Weimer
2022-08-01 12:49           ` Jason A. Donenfeld
2022-08-01 13:29             ` Jason A. Donenfeld
2022-08-01 13:00         ` Jason A. Donenfeld
2022-08-01 20:48         ` Thomas Gleixner
2022-08-01 23:41           ` Jason A. Donenfeld
2022-08-02  0:12             ` Jason A. Donenfeld
2022-08-01 19:30     ` [PATCH RFC v1] " Thomas Gleixner
2022-08-01 23:16       ` Jason A. Donenfeld [this message]
2022-08-02 13:46         ` Thomas Gleixner
2022-08-02 13:59           ` Jason A. Donenfeld
2022-08-02 15:14             ` Thomas Gleixner
2022-08-02 15:26               ` Jason A. Donenfeld
2022-08-02 22:27                 ` Thomas Gleixner
2022-08-04 15:23                   ` Jason A. Donenfeld
2022-08-04 16:08                     ` Jeffrey Walton
2022-08-04 23:11                     ` Thomas Gleixner
2022-08-17  8:20                   ` Peter Zijlstra
2022-08-05  8:36               ` Florian Weimer

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=Yuhe6IIFXqNMZs5b@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=fweimer@redhat.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nadiah@cs.ucsd.edu \
    --cc=ristenpart@cornell.edu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=vincenzo.frascino@arm.com \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).