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/[email protected]/T/
Babis Chalios (1):
vmgenid: emit uevent when VMGENID updates
drivers/virt/vmgenid.c | 2 ++
1 file changed, 2 insertions(+)
--
2.39.2
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 <[email protected]>
---
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
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/[email protected]/T/
>
> Babis Chalios (1):
> vmgenid: emit uevent when VMGENID updates
>
> drivers/virt/vmgenid.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Cheers,
Babis
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 <[email protected]>
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 <[email protected]>
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
On Mi, 31.05.23 11:51, Babis Chalios ([email protected]) 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 <[email protected]>
> ---
> 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 <[email protected]>
Lennart
--
Lennart Poettering, Berlin
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.
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
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
On Mo, 19.06.23 22:30, Jason A. Donenfeld ([email protected]) 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
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
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
On Wed, Jun 28, 2023 at 1:22 PM Greg KH <[email protected]> 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.
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 <[email protected]> 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
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
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
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