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

From: Lennart Poettering <mzxreary@0pointer.de>
To: Alexander Graf <graf@amazon.com>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	Dominik Brodowski <linux@dominikbrodowski.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Theodore Ts'o <tytso@mit.edu>,
	Colm MacCarthaigh <colmmacc@amazon.com>,
	Torben Hansen <htorben@amazon.co.uk>,
	Jann Horn <jannh@google.com>
Subject: Re: [PATCH 2/2] random: add fork_event sysctl for polling VM forks
Date: Tue, 3 May 2022 10:29:49 +0200	[thread overview]
Message-ID: <YnDn/d6iB0aUZkWJ@gardel-login> (raw)
In-Reply-To: <7a1cfd1c-9f0e-f134-e544-83ee6d3cd9c9@amazon.com>

On Mo, 02.05.22 19:59, Alexander Graf (graf@amazon.com) wrote:

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

I am not to keen on making sytemd such a proxy. Sounds like something
the kernel could do on its own, better and resulting in an ultimately
simpler system...

If systemd its the proxy this adds in extra raciness. i.e. in a ideal
world, if we have some form of notification fd, then it would be great
if that fd is guaranteed to have POLLIN set and its contents updated
the instant the clone happened. But if we proxy this through
userspace, there's necessarily a latency involved that it it takes
userspace to catch up and effect the POLLIN and updated contents
towards its client apps.

I understand the underlying VM hypervisor APIs currently are designed
to always imply some notification latency. Which sucks, but I think we
should be very careful with replicating this design mistake with any
userspace APIs we add.

i.e. I am pretty sure that even if the underlying VM hypervisor
primitive isn't as good as we wanted, the Linux kernel→userspace API
should be built so that if one day a better VM hypervisor interface
exists it can be plugged behind it without such limitations.

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

As mentioned earlier, I am not convinced sysctl is the right place for
this. sysctls are understood by most people as being the place for
tweaking kernel settings. This is not a kernel setting, but a
notification concept, and the way Jason defined it there's nothing to
read nor write, which strongly suggests to move it elsewhere, but not
/proc/sys/.

Use /sys/kernel/ or so. Or maybe O_NOTIFICATION_PIPE or whatever, but
/proc/sys/ looks really wrong.

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

I agree.

Lennart

--
Lennart Poettering, Berlin