Message-ID: <YnAiylnSytuYM53z@zx2c4.com>
Date:   Mon, 2 May 2022 20:29:11 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Alexander Graf <graf@...zon.com>
Cc:     Lennart Poettering <mzxreary@...inter.de>,
        linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Theodore Ts'o <tytso@....edu>,
        Colm MacCarthaigh <colmmacc@...zon.com>,
        Torben Hansen <htorben@...zon.co.uk>,
        Jann Horn <jannh@...gle.com>
Subject: Re: [PATCH 2/2] random: add fork_event sysctl for polling VM forks

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

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

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

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

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

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

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

Jason

Powered by blists - more mailing lists