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
