[PATCH 0/1] User space notifications about VM cloning

[PATCH 0/1] User space notifications about VM cloning

[Babis Chalios]

This patch revisits the story of user space notification about VM
events. It uses uevents to send a notification to user space that the
VMGENID has changed, thus we are now in a new VM.

Please note, that this is not a "you need to reseed your PRNGs" event,
which was what the previous RFC [1] was trying to do. It is, explicitly,
meant to be a "you are now running in a new VM" event for the user space
to consume, so it can do things like regenerating its MAC addresses and
refreshing DHCP. For these cases, we do not want to tie on the "you need
to reseed your PRNGs" event, since these do not necessarily get emitted
only when VMs get cloned.

[1] https://lore.kernel.org/lkml/65d872db2e1be29bb03b43ed606e7cc9e74ec08d.camel@infradead.org/T/

Babis Chalios (1):
  vmgenid: emit uevent when VMGENID updates

 drivers/virt/vmgenid.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.39.2

[PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Babis Chalios]

We receive an ACPI notification every time the VM Generation ID changes
and use the new ID as fresh randomness added to the entropy pool. This
commits emits a uevent every time we receive the ACPI notification, as a
means to notify the user space that it now is in a new VM.

Signed-off-by: Babis Chalios <bchalios@amazon.es>
---
 drivers/virt/vmgenid.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index a1c467a0e9f7..b67a28da4702 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -68,6 +68,7 @@ static int vmgenid_add(struct acpi_device *device)
 static void vmgenid_notify(struct acpi_device *device, u32 event)
 {
 	struct vmgenid_state *state = acpi_driver_data(device);
+	char *envp[] = { "NEW_VMGENID=1", NULL };
 	u8 old_id[VMGENID_SIZE];
 
 	memcpy(old_id, state->this_id, sizeof(old_id));
@@ -75,6 +76,7 @@ static void vmgenid_notify(struct acpi_device *device, u32 event)
 	if (!memcmp(old_id, state->this_id, sizeof(old_id)))
 		return;
 	add_vmfork_randomness(state->this_id, sizeof(state->this_id));
+	kobject_uevent_env(&device->dev.kobj, KOBJ_CHANGE, envp);
 }
 
 static const struct acpi_device_id vmgenid_ids[] = {
-- 
2.39.2

Re: [PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Alexander Graf]

On 31.05.23 11:51, Babis Chalios wrote:
> We receive an ACPI notification every time the VM Generation ID changes
> and use the new ID as fresh randomness added to the entropy pool. This
> commits emits a uevent every time we receive the ACPI notification, as a
> means to notify the user space that it now is in a new VM.
>
> Signed-off-by: Babis Chalios <bchalios@amazon.es>


Thanks Babis! Super simple, yet very effective way to notify system 
software that it may need to adopt to a new environment. I know that the 
systemd folks are super interested in that to for example regenerate 
randomly generated MAC addresses after a clone operation.


Reviewed-by: Alexander Graf <graf@amazon.com>


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

Re: [PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Lennart Poettering]

On Mi, 31.05.23 11:51, Babis Chalios (bchalios@amazon.es) wrote:

> We receive an ACPI notification every time the VM Generation ID changes
> and use the new ID as fresh randomness added to the entropy pool. This
> commits emits a uevent every time we receive the ACPI notification, as a
> means to notify the user space that it now is in a new VM.
>
> Signed-off-by: Babis Chalios <bchalios@amazon.es>
> ---
>  drivers/virt/vmgenid.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
> index a1c467a0e9f7..b67a28da4702 100644
> --- a/drivers/virt/vmgenid.c
> +++ b/drivers/virt/vmgenid.c
> @@ -68,6 +68,7 @@ static int vmgenid_add(struct acpi_device *device)
>  static void vmgenid_notify(struct acpi_device *device, u32 event)
>  {
>  	struct vmgenid_state *state = acpi_driver_data(device);
> +	char *envp[] = { "NEW_VMGENID=1", NULL };
>  	u8 old_id[VMGENID_SIZE];
>
>  	memcpy(old_id, state->this_id, sizeof(old_id));
> @@ -75,6 +76,7 @@ static void vmgenid_notify(struct acpi_device *device, u32 event)
>  	if (!memcmp(old_id, state->this_id, sizeof(old_id)))
>  		return;
>  	add_vmfork_randomness(state->this_id, sizeof(state->this_id));
> +	kobject_uevent_env(&device->dev.kobj, KOBJ_CHANGE, envp);
>  }
>
>  static const struct acpi_device_id vmgenid_ids[] = {

Beautifully simple. Looks good to me. Would love to make use of this from systemd.

Reviewed-by: Lennart Poettering <mzxreary@0pointer.de>

Lennart

--
Lennart Poettering, Berlin

Re: [PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Jason A. Donenfeld]

Like the other patch, and as discussed before too, I don't think this
has any business being part of (virtual) hardware drivers, and instead
belongs in random.c, which might receive these notifications from a
variety of devices, and can thus synchronize things accordingly.
Please stop posting more of these same approaches. Same nack as the
other ones.

Re: [PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Alexander Graf]

Hey Jason,

On 19.06.23 22:30, Jason A. Donenfeld wrote:
> Like the other patch, and as discussed before too, I don't think this
> has any business being part of (virtual) hardware drivers, and instead
> belongs in random.c, which might receive these notifications from a
> variety of devices, and can thus synchronize things accordingly.
> Please stop posting more of these same approaches. Same nack as the
> other ones.


Could you please elaborate what other devices you envision emitting 
"This VM was cloned, you MAC address may now collide" style events?

What we talked about at LPC was an orthogonal interface that allows user 
space to receive reseed events when either the kernel, an RNG device or 
anything else in the system wants to say "Your cached randomness may be 
compromised, please fetch some new".

This patch is not that interface. It's an event meant for systemd (and 
other system software) to know exclusively about VM clone events. That 
system software can not use the reseed event above: Just imagine getting 
a new MAC address every 5 minutes. So here we really just want to know 
the vmgenid changed, no more, no less.


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

Re: [PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Babis Chalios]

Hi Jason,

On 19/6/23 22:30, Jason A. Donenfeld wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> Like the other patch, and as discussed before too, I don't think this
> has any business being part of (virtual) hardware drivers, and instead
> belongs in random.c, which might receive these notifications from a
> variety of devices, and can thus synchronize things accordingly.
> Please stop posting more of these same approaches. Same nack as the
> other ones.

Quoting the cover letter of this patchset

> Please note, that this is not a "you need to reseed your PRNGs" event,
> which was what the previous RFC [1] was trying to do. It is, explicitly,
> meant to be a "you are now running in a new VM" event for the user space
> to consume, so it can do things like regenerating its MAC addresses and
> refreshing DHCP.

Why do you think that the "you are now running in a new VM" event (that has
nothing to do with PRNGs) belongs in random.c?

Cheers,
Babis

Re: [PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Lennart Poettering]

On Mo, 19.06.23 22:30, Jason A. Donenfeld (Jason@zx2c4.com) wrote:

> Like the other patch, and as discussed before too, I don't think this
> has any business being part of (virtual) hardware drivers, and instead
> belongs in random.c, which might receive these notifications from a
> variety of devices, and can thus synchronize things accordingly.
> Please stop posting more of these same approaches. Same nack as the
> other ones.

Note the intended usecase for this in userspace really has nothing to
do with RNGs. We just want an event that is generated when a machine
is duplicated so that we can request a new DHCP lease, and similar. I
don't see any relationship to random.c for that.

Lennart

--
Lennart Poettering, Berlin

Re: [PATCH 1/1] vmgenid: emit uevent when VMGENID updates

[Alexander Graf]

Hi Jason,

On 20.06.23 13:28, Lennart Poettering wrote:
> On Mo, 19.06.23 22:30, Jason A. Donenfeld (Jason@zx2c4.com) wrote:
>
>> Like the other patch, and as discussed before too, I don't think this
>> has any business being part of (virtual) hardware drivers, and instead
>> belongs in random.c, which might receive these notifications from a
>> variety of devices, and can thus synchronize things accordingly.
>> Please stop posting more of these same approaches. Same nack as the
>> other ones.
> Note the intended usecase for this in userspace really has nothing to
> do with RNGs. We just want an event that is generated when a machine
> is duplicated so that we can request a new DHCP lease, and similar. I
> don't see any relationship to random.c for that.


Checking up on this conversation. The intended use case has nothing to 
do with RNG - we merely need a notification for system level user space 
that a system clone operation happened so it can act on it.

Please consider taking the patch as is or advise on how to proceed to 
address this non-RNG use case.


Thanks,

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

Re: [PATCH 0/1] User space notifications about VM cloning

[Babis Chalios]

Hello all,

Some time has passed since I sent this. Any comments/thoughts?

On 31/5/23 11:51, Babis Chalios wrote:
> This patch revisits the story of user space notification about VM
> events. It uses uevents to send a notification to user space that the
> VMGENID has changed, thus we are now in a new VM.
>
> Please note, that this is not a "you need to reseed your PRNGs" event,
> which was what the previous RFC [1] was trying to do. It is, explicitly,
> meant to be a "you are now running in a new VM" event for the user space
> to consume, so it can do things like regenerating its MAC addresses and
> refreshing DHCP. For these cases, we do not want to tie on the "you need
> to reseed your PRNGs" event, since these do not necessarily get emitted
> only when VMs get cloned.
>
> [1] https://lore.kernel.org/lkml/65d872db2e1be29bb03b43ed606e7cc9e74ec08d.camel@infradead.org/T/
>
> Babis Chalios (1):
>    vmgenid: emit uevent when VMGENID updates
>
>   drivers/virt/vmgenid.c | 2 ++
>   1 file changed, 2 insertions(+)
>

Cheers,
Babis

Re: [PATCH 0/1] User space notifications about VM cloning

[Alexander Graf]

Hi folks,

On 16.06.23 17:07, Babis Chalios wrote:
> Hello all,
>
> Some time has passed since I sent this. Any comments/thoughts?


Can we please get this merged somehow? Greg, any advise?

This is purely a device notification event to user space, similar to 
network link change events and the likes and has nothing to do with 
Jason's envisioned random reseed event exposure. We can happily send RFC 
patches for the latter after this is merged too.


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

Re: [PATCH 0/1] User space notifications about VM cloning

[Greg KH]

On Wed, Jun 28, 2023 at 01:13:40PM +0200, Alexander Graf wrote:
> Hi folks,
> 
> On 16.06.23 17:07, Babis Chalios wrote:
> > Hello all,
> > 
> > Some time has passed since I sent this. Any comments/thoughts?
> 
> 
> Can we please get this merged somehow? Greg, any advise?
> 
> This is purely a device notification event to user space, similar to network
> link change events and the likes and has nothing to do with Jason's
> envisioned random reseed event exposure. We can happily send RFC patches for
> the latter after this is merged too.

Sure, I can take it, but it's the middle of the merge window and it's
too late for anything new right now, sorry.

I'll pick it up after 6.5-rc1 is out.

thanks,

greg k-h

Re: [PATCH 0/1] User space notifications about VM cloning

[Jason A. Donenfeld]

On Wed, Jun 28, 2023 at 1:22 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 28, 2023 at 01:13:40PM +0200, Alexander Graf wrote:
> > Hi folks,
> >
> > On 16.06.23 17:07, Babis Chalios wrote:
> > > Hello all,
> > >
> > > Some time has passed since I sent this. Any comments/thoughts?
> >
> >
> > Can we please get this merged somehow? Greg, any advise?
> >
> > This is purely a device notification event to user space, similar to network
> > link change events and the likes and has nothing to do with Jason's
> > envisioned random reseed event exposure. We can happily send RFC patches for
> > the latter after this is merged too.
>
> Sure, I can take it, but it's the middle of the merge window and it's
> too late for anything new right now, sorry.
>
> I'll pick it up after 6.5-rc1 is out.

Please do *NOT* do that. I'm still unconvinced that this isn't just a
subset of the vmclone work that needs to be done in conjunction with
the RNG, and I'd like to get those recent virtio patches merged first
before we move onto this, so we can see where this fits in
holistically. I would not be happy if this got merged prematurely.

Re: [PATCH 0/1] User space notifications about VM cloning

[Greg KH]

On Wed, Jun 28, 2023 at 01:36:51PM +0200, Jason A. Donenfeld wrote:
> On Wed, Jun 28, 2023 at 1:22 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jun 28, 2023 at 01:13:40PM +0200, Alexander Graf wrote:
> > > Hi folks,
> > >
> > > On 16.06.23 17:07, Babis Chalios wrote:
> > > > Hello all,
> > > >
> > > > Some time has passed since I sent this. Any comments/thoughts?
> > >
> > >
> > > Can we please get this merged somehow? Greg, any advise?
> > >
> > > This is purely a device notification event to user space, similar to network
> > > link change events and the likes and has nothing to do with Jason's
> > > envisioned random reseed event exposure. We can happily send RFC patches for
> > > the latter after this is merged too.
> >
> > Sure, I can take it, but it's the middle of the merge window and it's
> > too late for anything new right now, sorry.
> >
> > I'll pick it up after 6.5-rc1 is out.
> 
> Please do *NOT* do that. I'm still unconvinced that this isn't just a
> subset of the vmclone work that needs to be done in conjunction with
> the RNG, and I'd like to get those recent virtio patches merged first
> before we move onto this, so we can see where this fits in
> holistically. I would not be happy if this got merged prematurely.

Ok, will hold off on this until you all work it out.  The changelog text
discusses entropy, so it makes sense that this looks like it is related
to this topic.

thanks,

greg k-h

Re: [PATCH 0/1] User space notifications about VM cloning

[Greg KH]

On Wed, Jun 28, 2023 at 01:22:11PM +0200, Greg KH wrote:
> On Wed, Jun 28, 2023 at 01:13:40PM +0200, Alexander Graf wrote:
> > Hi folks,
> > 
> > On 16.06.23 17:07, Babis Chalios wrote:
> > > Hello all,
> > > 
> > > Some time has passed since I sent this. Any comments/thoughts?
> > 
> > 
> > Can we please get this merged somehow? Greg, any advise?
> > 
> > This is purely a device notification event to user space, similar to network
> > link change events and the likes and has nothing to do with Jason's
> > envisioned random reseed event exposure. We can happily send RFC patches for
> > the latter after this is merged too.
> 
> Sure, I can take it, but it's the middle of the merge window and it's
> too late for anything new right now, sorry.
> 
> I'll pick it up after 6.5-rc1 is out.

Wait, no, I'm not the maintainer of this, Jason is.  And he already
rejected it (and based on the changelog text, I would too), so why are
you asking me a month later to take this?

Work with the maintainer please, don't try to route around them, you
both know better than this.

greg k-h

Re: [PATCH 0/1] User space notifications about VM cloning

[Jason A. Donenfeld]

Just so you guys know, roughly the order of operations here are going to be:

- vdso vgetrandom v+1
- virtio fork driver
- exposing fork events to userspace

I'll keep you posted on those.

Jason

Re: [PATCH 0/1] User space notifications about VM cloning

[Amit Shah]

On Wed, 2023-06-28 at 18:27 +0200, Jason A. Donenfeld wrote:
> Just so you guys know, roughly the order of operations here are going to be:
> 
> - vdso vgetrandom v+1
> - virtio fork driver
> - exposing fork events to userspace
> 
> I'll keep you posted on those.

Thank you!

One of the things I've struggled with is the lack of updates or
direction from Jason to Babis - he's had patches out for a while, and
Jason has said he's going to drive it, but we didn't see follow-ups.

At least this conversation has the signs of progress.

Thanks!

		Amit