Re: [PATCH 2/2] random: add fork_event sysctl for polling VM forks

[Alexander Graf <graf@amazon.com>]

Hey Jason,

On 02.05.22 20:29, Jason A. Donenfeld wrote:
> Hi Alex,
>
> On Mon, May 02, 2022 at 07:59:08PM +0200, Alexander Graf wrote:
>> to collect the use cases we all have and evaluate whether this patch is
>> a good stepping stone towards the final solution.
> Indeed, I'm all for collecting use cases. What I meant to say is that
> we're not going to add something "just 'cuz"; I'd like to have some
> concrete things in mind.
>
> To date, I've basically had your s2n case in mind, but as you haven't
> responded to this in the last month, I started looking to see if this


Unfortunate vacation timing on my side I suppose :)


> was useful elsewhere or if I should abandon it, so I filed this issue
> with the Go project: <https://github.com/golang/go/issues/52544>. We're
> over halfway through 5.18 now, and only at this point have you arrived
> to discuss and finalize things. So in all likelihood we'll wind up
> tabling this until 5.20 or never, since what I thought was an easy
> consensus before now apparently is not.


So far I see little that would block your patch? It seems to go into a 
good direction from all I can tell.


>> 1) A way for libraries such as s2n to identify that a clone occurred.
>> Because it's a deep-down library with no access to its own thread or the
>> main loop, it can not rely on poll/select. Mmap of a file however would
>> work great, as you can create transactions on top of a 64bit mmap'ed
>> value for example.
> I didn't realize that s2n can't poll. That's surprising. In the worst
> case, can't you just spawn a thread?


You block the thread on poll, so the only option is a thread. I was 
until now always under the working assumption that we can't do this in a 
thread because you don't want your single threaded application to turn 
into a pthreaded one, but you make me wonder. Let me check with Torben 
tomorrow.


>
>> 2) A way to notify larger applications (think Java here) that a system
>> is going to be suspended soon so it can wipe PII before it gets cloned
>> for example.
> Suspension, like S3 power notification stuff? Talk to Rafael about that;


Whether you go running -> S3 -> clone or you go running -> paused -> 
clone is an implementation detail I'm not terribly worried about. Users 
can do either, because on both cases the VM is in paused state.


> this isn't related to the VM fork issue. I use those PM notifiers
> happily in kernel space but AFAICT, there's still no userspace thing for
> it. This seems orthogonal to this conversation though, so let's not veer
> off into that topic.
>
> If you didn't mean S3 but actually meant notification prior to snapshot
> taking, we don't have any virtual hardware for that, so it's a moot
> point.


I think we'll want to have an external button similar to the ACPI sleep 
button or lid close event eventually, so that we can loop the VM in on 
the suspend path the same way S3 does.

Today we don't have it, I agree. And if it's possible to maintain the 
same user space interface with this in mind, all is good. Let's just 
keep it in mind as something that will probably come eventually so that 
we don't redesign the UAPI in a year from now.


>
>> 3) Notifications after clone so applications know they can regenerate VM
>> unique data based on randomness.
> You mean this as "the same as (1) but with poll() instead of mmap()",
> right?


Yes :)


>
>> Lennart, looking at the current sysctl proposal, systemd could poll() on
>> the fork file. It would then be able to generate a /run/fork-id file
>> which it can use for the flow above, right?
> For the reasons I gave in my last email to Lennart, I don't think
> there's a good way for systemd to generate a fork-id file on its own
> either. I don't think systemd should really be involved here as a
> provider of values, just as a potential consumer of what the kernel
> provides.


Yes, systemd would poll on fork_event. When that returns, it reads 
/dev/urandom and writes a new /run/fork-id file with that. Libraries 
that don't want to be in the business of spawning threads can use that 
file to identify that they were cloned. Systemd can use that id as seed 
for networkd.


>
>> The sysctl proposal also gives us 3, if we implement the inhibitor
>> proposal [1] in systemd.
> These userspace components you're proposing seem like a lot of
> overengineering for little gain, which is why this conversation went
> nowhere when Amazon attempted all this year. But it sounds like you
> agree with me based on your remark below about systemd-less interfaces
> provided by the kernel.


I would be happy to get to 99% of this with pure kernel based 
interfaces. The reason the kernel conversation seemingly went nowhere 
was not because of "overengineering". It was because the review feedback 
eventually was "This is a file, make user space manage it".


>> Overall, it sounds to me like the sysctl poll based kernel interface in
>> this patch in combination with systemd inhibitors gives us an answer to
>> most of the flows above.
>>
>> I can see attractiveness in providing the /run/fork-id directly from the
>> kernel though, to remove the dependency on systemd for poll-less
>> notification of libraries.
>>
>> Jason, how much complexity would it add to provide an mmap() and read()
>> interface to a fork counter value to the sysctl? Read sounds like a
>> trivial change on top of what you have already, mmap a bit more heavy
>> lift. If we had both, it would allow us to implement a Linux standard
>> fork detect path in libraries that does not rely on systemd.
> mmap() does not give us anything if we're not going to expose the raw
> ACPI-mapped ID directly. It will still be a racy mechanism until we do
> that. So I think we should wait until there's a proper vmgenid
> word-sized counter to expose something mmap()able. If you have the
> energy to talk to Microsoft about this and make it happen, please be my
> guest. As I wrote at the beginning of this email. I haven't gotten a
> response from you at all about this stuff in quite some time, so I'm not
> really itching take that on alone now.


Absolutely! Let's see where it goes :)


Alex





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879