| Message ID | 20220301173120.297105-1-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | PM: notify of PM_POST_VMFORK events from vmgenid | expand |
On Tue, Mar 1, 2022 at 6:31 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > There's an additional virtual power state that various crypto-oriented > drivers may benefit from being notified of, such as WireGuard: right > after a virtual machine has forked. In WireGuard's case, the PM notifier > there that clears keys pre-suspend will be adjusted to also clear them > post-vmfork. This trivial commit wires up the machinery for that change, > which builds on the recently added vmgenid driver in the random.git > tree. Well, what does power management have to do with WireGuard'? > Cc: Rafael J. Wysocki <rafael@kernel.org> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Len Brown <len.brown@intel.com> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > drivers/virt/vmgenid.c | 2 ++ > include/linux/suspend.h | 4 ++++ > kernel/power/main.c | 6 ++++++ > 3 files changed, 12 insertions(+) > > diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c > index 0ae1a39f2e28..4bef3e8b3476 100644 > --- a/drivers/virt/vmgenid.c > +++ b/drivers/virt/vmgenid.c > @@ -10,6 +10,7 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/acpi.h> > +#include <linux/suspend.h> > #include <linux/random.h> > > ACPI_MODULE_NAME("vmgenid"); > @@ -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)); > + pm_notify_vmfork(); IMV, this is completely confusing. > } > > static const struct acpi_device_id vmgenid_ids[] = { > diff --git a/include/linux/suspend.h b/include/linux/suspend.h > index 300273ff40cc..a3836473b87b 100644 > --- a/include/linux/suspend.h > +++ b/include/linux/suspend.h > @@ -477,6 +477,7 @@ static inline int is_hibernate_resume_dev(dev_t dev) { return 0; } > #define PM_POST_SUSPEND 0x0004 /* Suspend finished */ > #define PM_RESTORE_PREPARE 0x0005 /* Going to restore a saved image */ > #define PM_POST_RESTORE 0x0006 /* Restore failed */ > +#define PM_POST_VMFORK 0x0007 /* Virtual machine has just forked */ > > extern struct mutex system_transition_mutex; > > @@ -487,6 +488,7 @@ void restore_processor_state(void); > /* kernel/power/main.c */ > extern int register_pm_notifier(struct notifier_block *nb); > extern int unregister_pm_notifier(struct notifier_block *nb); > +extern void pm_notify_vmfork(void); > extern void ksys_sync_helper(void); > > #define pm_notifier(fn, pri) { \ > @@ -525,6 +527,8 @@ static inline int unregister_pm_notifier(struct notifier_block *nb) > return 0; > } > > +static inline void pm_notify_vmfork(void) {} > + > static inline void ksys_sync_helper(void) {} > > #define pm_notifier(fn, pri) do { (void)(fn); } while (0) > diff --git a/kernel/power/main.c b/kernel/power/main.c > index 7e646079fbeb..a64cfb36b1b3 100644 > --- a/kernel/power/main.c > +++ b/kernel/power/main.c > @@ -94,6 +94,12 @@ int pm_notifier_call_chain(unsigned long val) > return blocking_notifier_call_chain(&pm_chain_head, val, NULL); > } > > +void pm_notify_vmfork(void) > +{ > + pm_notifier_call_chain(PM_POST_VMFORK); > +} > +EXPORT_SYMBOL_GPL(pm_notify_vmfork); > + > /* If set, devices may be suspended and resumed asynchronously. */ > int pm_async_enabled = 1; > > -- > 2.35.1 >
Hi Rafael, On Tue, Mar 1, 2022 at 6:36 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Tue, Mar 1, 2022 at 6:31 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > There's an additional virtual power state that various crypto-oriented > > drivers may benefit from being notified of, such as WireGuard: right > > after a virtual machine has forked. In WireGuard's case, the PM notifier > > there that clears keys pre-suspend will be adjusted to also clear them > > post-vmfork. This trivial commit wires up the machinery for that change, > > which builds on the recently added vmgenid driver in the random.git > > tree. > > Well, what does power management have to do with WireGuard'? I guess a bit more background would be in order. If I post a v2 of this, I'll include that there. But for now: WireGuard has ephemeral session keys. They're not supposed to exist for longer than a few minutes for a crypto property called "forward secrecy". In order to ensure this, WireGuard currently registers a PM notifier that fires before suspend/hibernate, which memzeros the various keys. That's all well and fine and works. There's now another power-ish event that WireGuard also cares about: when a virtual machine has been forked. In this case, too, the reaction is the same - memzero the various keys, only for a different reason: rather than forward secrecy, the property we want here is that a key+nonce tuple is never used on more than one plaintext. The argument of this patchset is that VM forking is kind of like a power event, so why not re-use the same notifier for that. However, if you disagree, I could move ahead with a separate notification mechanism not involving the PM notifier. Jason
On Tue, Mar 1, 2022 at 7:19 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Rafael, > > On Tue, Mar 1, 2022 at 6:36 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Tue, Mar 1, 2022 at 6:31 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > > > There's an additional virtual power state that various crypto-oriented > > > drivers may benefit from being notified of, such as WireGuard: right > > > after a virtual machine has forked. In WireGuard's case, the PM notifier > > > there that clears keys pre-suspend will be adjusted to also clear them > > > post-vmfork. This trivial commit wires up the machinery for that change, > > > which builds on the recently added vmgenid driver in the random.git > > > tree. > > > > Well, what does power management have to do with WireGuard'? > > I guess a bit more background would be in order. If I post a v2 of > this, I'll include that there. But for now: > > WireGuard has ephemeral session keys. They're not supposed to exist > for longer than a few minutes for a crypto property called "forward > secrecy". In order to ensure this, WireGuard currently registers a PM > notifier that fires before suspend/hibernate, which memzeros the > various keys. That's all well and fine and works. > > There's now another power-ish event that WireGuard also cares about: > when a virtual machine has been forked. In this case, too, the > reaction is the same - memzero the various keys, only for a different > reason: rather than forward secrecy, the property we want here is that > a key+nonce tuple is never used on more than one plaintext. > > The argument of this patchset is that VM forking is kind of like a > power event, so why not re-use the same notifier for that. IMV the problem with this approach is that WireGuard is not the only user of PM (suspend/hibernate) notifiers and the other users of them will have no idea about what to do with PM_POST_VMFORK which from their perspective has nothing to do with suspend and/or hibernation. > However, if you disagree, I could move ahead with a separate notification > mechanism not involving the PM notifier. Yes, please.
On Tue, Mar 1, 2022 at 7:50 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Tue, Mar 1, 2022 at 7:19 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > Hi Rafael, > > > > On Tue, Mar 1, 2022 at 6:36 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > > > On Tue, Mar 1, 2022 at 6:31 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > > > > > There's an additional virtual power state that various crypto-oriented > > > > drivers may benefit from being notified of, such as WireGuard: right > > > > after a virtual machine has forked. In WireGuard's case, the PM notifier > > > > there that clears keys pre-suspend will be adjusted to also clear them > > > > post-vmfork. This trivial commit wires up the machinery for that change, > > > > which builds on the recently added vmgenid driver in the random.git > > > > tree. > > > > > > Well, what does power management have to do with WireGuard'? > > > > I guess a bit more background would be in order. If I post a v2 of > > this, I'll include that there. But for now: > > > > WireGuard has ephemeral session keys. They're not supposed to exist > > for longer than a few minutes for a crypto property called "forward > > secrecy". In order to ensure this, WireGuard currently registers a PM > > notifier that fires before suspend/hibernate, which memzeros the > > various keys. That's all well and fine and works. > > > > There's now another power-ish event that WireGuard also cares about: > > when a virtual machine has been forked. In this case, too, the > > reaction is the same - memzero the various keys, only for a different > > reason: rather than forward secrecy, the property we want here is that > > a key+nonce tuple is never used on more than one plaintext. > > > > The argument of this patchset is that VM forking is kind of like a > > power event, so why not re-use the same notifier for that. > > IMV the problem with this approach is that WireGuard is not the only > user of PM (suspend/hibernate) notifiers and the other users of them > will have no idea about what to do with PM_POST_VMFORK which from > their perspective has nothing to do with suspend and/or hibernation. > > > However, if you disagree, I could move ahead with a separate notification > > mechanism not involving the PM notifier. > > Yes, please. Will do. Sorry for the noise.
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c index 0ae1a39f2e28..4bef3e8b3476 100644 --- a/drivers/virt/vmgenid.c +++ b/drivers/virt/vmgenid.c @@ -10,6 +10,7 @@ #include <linux/kernel.h> #include <linux/module.h> #include <linux/acpi.h> +#include <linux/suspend.h> #include <linux/random.h> ACPI_MODULE_NAME("vmgenid"); @@ -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)); + pm_notify_vmfork(); } static const struct acpi_device_id vmgenid_ids[] = { diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 300273ff40cc..a3836473b87b 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -477,6 +477,7 @@ static inline int is_hibernate_resume_dev(dev_t dev) { return 0; } #define PM_POST_SUSPEND 0x0004 /* Suspend finished */ #define PM_RESTORE_PREPARE 0x0005 /* Going to restore a saved image */ #define PM_POST_RESTORE 0x0006 /* Restore failed */ +#define PM_POST_VMFORK 0x0007 /* Virtual machine has just forked */ extern struct mutex system_transition_mutex; @@ -487,6 +488,7 @@ void restore_processor_state(void); /* kernel/power/main.c */ extern int register_pm_notifier(struct notifier_block *nb); extern int unregister_pm_notifier(struct notifier_block *nb); +extern void pm_notify_vmfork(void); extern void ksys_sync_helper(void); #define pm_notifier(fn, pri) { \ @@ -525,6 +527,8 @@ static inline int unregister_pm_notifier(struct notifier_block *nb) return 0; } +static inline void pm_notify_vmfork(void) {} + static inline void ksys_sync_helper(void) {} #define pm_notifier(fn, pri) do { (void)(fn); } while (0) diff --git a/kernel/power/main.c b/kernel/power/main.c index 7e646079fbeb..a64cfb36b1b3 100644 --- a/kernel/power/main.c +++ b/kernel/power/main.c @@ -94,6 +94,12 @@ int pm_notifier_call_chain(unsigned long val) return blocking_notifier_call_chain(&pm_chain_head, val, NULL); } +void pm_notify_vmfork(void) +{ + pm_notifier_call_chain(PM_POST_VMFORK); +} +EXPORT_SYMBOL_GPL(pm_notify_vmfork); + /* If set, devices may be suspended and resumed asynchronously. */ int pm_async_enabled = 1;
There's an additional virtual power state that various crypto-oriented drivers may benefit from being notified of, such as WireGuard: right after a virtual machine has forked. In WireGuard's case, the PM notifier there that clears keys pre-suspend will be adjusted to also clear them post-vmfork. This trivial commit wires up the machinery for that change, which builds on the recently added vmgenid driver in the random.git tree. Cc: Rafael J. Wysocki <rafael@kernel.org> Cc: Pavel Machek <pavel@ucw.cz> Cc: Len Brown <len.brown@intel.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- drivers/virt/vmgenid.c | 2 ++ include/linux/suspend.h | 4 ++++ kernel/power/main.c | 6 ++++++ 3 files changed, 12 insertions(+)