Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>
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
In-Reply-To: <Yuhe6IIFXqNMZs5b@zx2c4.com>
References: <20220729145525.1729066-1-Jason@zx2c4.com>
 <CAHk-=wiLwz=9h9LD1-_yb1+T+u59a2EjTmMvCiGj4A-ZsPN1wA@mail.gmail.com>
 <YuXCpyULk6jFgGV5@zx2c4.com> <87zggnsqwj.ffs@tglx>
 <Yuhe6IIFXqNMZs5b@zx2c4.com>
Date:   Tue, 02 Aug 2022 15:46:27 +0200
Message-ID: <87bkt2sqq4.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Jason!

On Tue, Aug 02 2022 at 01:16, Jason A. Donenfeld wrote:
> On Mon, Aug 01, 2022 at 09:30:20PM +0200, Thomas Gleixner wrote:
>> > 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:

I did not miss this. I carefully read it.

> 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.

I surely understand your goal, but the real question is where we draw
the line and what kind of functionality should go into such a library
and what's the required justification for it.

The concept of system libraries on Windows NT was to provide different
APIs for application programmers: Win32, OS/2, Posix. That allowed to
change the actual syscalls without breaking existing applications. IOW,
it's just a glue layer which translates between application API and
syscall API.

Right now the Linux VDSO functions are 1:1 replacements for system calls
and not adding a magic pile of functionality which is otherwise not
available.

What you are proposing is to have an implementation which is not
available via a regular syscall. Which means you are creating a VDSO
only syscall which still has the same problem as any other syscall in
terms of API design and functionality which needs to be supported
forever.

Thanks,

        tglx