Re: [PATCH v19 6/8] PM: hibernate: disable when there are active secretmem users
On Tue, 2021-05-18 at 18:49 -0700, Dan Williams wrote: > On Tue, May 18, 2021 at 6:33 PM James Bottomley > wrote: > > On Tue, 2021-05-18 at 11:24 +0100, Mark Rutland wrote: > > > On Thu, May 13, 2021 at 09:47:32PM +0300, Mike Rapoport wrote: > > > > From: Mike Rapoport > > > > > > > > It is unsafe to allow saving of secretmem areas to the > > > > hibernation snapshot as they would be visible after the resume > > > > and this essentially will defeat the purpose of secret memory > > > > mappings. > > > > > > > > Prevent hibernation whenever there are active secret memory > > > > users. > > > > > > Have we thought about how this is going to work in practice, e.g. > > > on mobile systems? It seems to me that there are a variety of > > > common applications which might want to use this which people > > > don't expect to inhibit hibernate (e.g. authentication agents, > > > web browsers). > > > > If mobile systems require hibernate, then the choice is to disable > > this functionality or implement a secure hibernation store. I > > also thought most mobile hibernation was basically equivalent to > > S3, in which case there's no actual writing of ram into storage, in > > which case there's no security barrier and likely the inhibition > > needs to be made a bit more specific to the suspend to disk case? > > > > > Are we happy to say that any userspace application can > > > incidentally inhibit hibernate? > > > > Well, yes, for the laptop use case because we don't want suspend to > > disk to be able to compromise the secret area. You can disable > > this for mobile if you like, or work out how to implement hibernate > > securely if you're really suspending to disk. > > Forgive me if this was already asked and answered. Why not document > that secretmem is ephemeral in the case of hibernate and push the > problem to userspace to disable hibernation? In other words > hibernation causes applications to need to reload their secretmem, it > will be destroyed on the way down and SIGBUS afterwards. That at > least gives a system the flexibility to either sacrifice hibernate > for secretmem (with a userspace controlled policy), or sacrifice > secretmem using processes for hibernate. Well, realistically, there are many possibilities for embedded if it wants to use secret memory. However, not really having much of an interest in the use cases, it's not really for Mike or me to be acting as armchair fly half. I think the best we can do is demonstrate the system for our use cases and let embedded kick the tyres for theirs if they care, and if not they can disable the feature. James ___ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [PATCH v19 6/8] PM: hibernate: disable when there are active secretmem users
On Tue, May 18, 2021 at 6:33 PM James Bottomley wrote: > > On Tue, 2021-05-18 at 11:24 +0100, Mark Rutland wrote: > > On Thu, May 13, 2021 at 09:47:32PM +0300, Mike Rapoport wrote: > > > From: Mike Rapoport > > > > > > It is unsafe to allow saving of secretmem areas to the hibernation > > > snapshot as they would be visible after the resume and this > > > essentially will defeat the purpose of secret memory mappings. > > > > > > Prevent hibernation whenever there are active secret memory users. > > > > Have we thought about how this is going to work in practice, e.g. on > > mobile systems? It seems to me that there are a variety of common > > applications which might want to use this which people don't expect > > to inhibit hibernate (e.g. authentication agents, web browsers). > > If mobile systems require hibernate, then the choice is to disable this > functionality or implement a secure hibernation store. I also thought > most mobile hibernation was basically equivalent to S3, in which case > there's no actual writing of ram into storage, in which case there's no > security barrier and likely the inhibition needs to be made a bit more > specific to the suspend to disk case? > > > Are we happy to say that any userspace application can incidentally > > inhibit hibernate? > > Well, yes, for the laptop use case because we don't want suspend to > disk to be able to compromise the secret area. You can disable this > for mobile if you like, or work out how to implement hibernate securely > if you're really suspending to disk. Forgive me if this was already asked and answered. Why not document that secretmem is ephemeral in the case of hibernate and push the problem to userspace to disable hibernation? In other words hibernation causes applications to need to reload their secretmem, it will be destroyed on the way down and SIGBUS afterwards. That at least gives a system the flexibility to either sacrifice hibernate for secretmem (with a userspace controlled policy), or sacrifice secretmem using processes for hibernate. ___ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [PATCH v19 6/8] PM: hibernate: disable when there are active secretmem users
On Tue, 2021-05-18 at 11:24 +0100, Mark Rutland wrote: > On Thu, May 13, 2021 at 09:47:32PM +0300, Mike Rapoport wrote: > > From: Mike Rapoport > > > > It is unsafe to allow saving of secretmem areas to the hibernation > > snapshot as they would be visible after the resume and this > > essentially will defeat the purpose of secret memory mappings. > > > > Prevent hibernation whenever there are active secret memory users. > > Have we thought about how this is going to work in practice, e.g. on > mobile systems? It seems to me that there are a variety of common > applications which might want to use this which people don't expect > to inhibit hibernate (e.g. authentication agents, web browsers). If mobile systems require hibernate, then the choice is to disable this functionality or implement a secure hibernation store. I also thought most mobile hibernation was basically equivalent to S3, in which case there's no actual writing of ram into storage, in which case there's no security barrier and likely the inhibition needs to be made a bit more specific to the suspend to disk case? > Are we happy to say that any userspace application can incidentally > inhibit hibernate? Well, yes, for the laptop use case because we don't want suspend to disk to be able to compromise the secret area. You can disable this for mobile if you like, or work out how to implement hibernate securely if you're really suspending to disk. James ___ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [PATCH v19 6/8] PM: hibernate: disable when there are active secretmem users
On 18.05.21 12:24, Mark Rutland wrote: On Thu, May 13, 2021 at 09:47:32PM +0300, Mike Rapoport wrote: From: Mike Rapoport It is unsafe to allow saving of secretmem areas to the hibernation snapshot as they would be visible after the resume and this essentially will defeat the purpose of secret memory mappings. Prevent hibernation whenever there are active secret memory users. Have we thought about how this is going to work in practice, e.g. on mobile systems? It seems to me that there are a variety of common applications which might want to use this which people don't expect to inhibit hibernate (e.g. authentication agents, web browsers). Are we happy to say that any userspace application can incidentally inhibit hibernate? It's worth noting that secretmem has to be explicitly enabled by the admin to even work. -- Thanks, David / dhildenb ___ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [PATCH v19 6/8] PM: hibernate: disable when there are active secretmem users
On Thu, May 13, 2021 at 09:47:32PM +0300, Mike Rapoport wrote:
> From: Mike Rapoport
>
> It is unsafe to allow saving of secretmem areas to the hibernation
> snapshot as they would be visible after the resume and this essentially
> will defeat the purpose of secret memory mappings.
>
> Prevent hibernation whenever there are active secret memory users.
Have we thought about how this is going to work in practice, e.g. on
mobile systems? It seems to me that there are a variety of common
applications which might want to use this which people don't expect to
inhibit hibernate (e.g. authentication agents, web browsers).
Are we happy to say that any userspace application can incidentally
inhibit hibernate?
Thanks,
Mark.
> Signed-off-by: Mike Rapoport
> Cc: Alexander Viro
> Cc: Andy Lutomirski
> Cc: Arnd Bergmann
> Cc: Borislav Petkov
> Cc: Catalin Marinas
> Cc: Christopher Lameter
> Cc: Dan Williams
> Cc: Dave Hansen
> Cc: David Hildenbrand
> Cc: Elena Reshetova
> Cc: Hagen Paul Pfeifer
> Cc: "H. Peter Anvin"
> Cc: Ingo Molnar
> Cc: James Bottomley
> Cc: "Kirill A. Shutemov"
> Cc: Mark Rutland
> Cc: Matthew Wilcox
> Cc: Michael Kerrisk
> Cc: Palmer Dabbelt
> Cc: Palmer Dabbelt
> Cc: Paul Walmsley
> Cc: Peter Zijlstra
> Cc: Rick Edgecombe
> Cc: Roman Gushchin
> Cc: Shakeel Butt
> Cc: Shuah Khan
> Cc: Thomas Gleixner
> Cc: Tycho Andersen
> Cc: Will Deacon
> ---
> include/linux/secretmem.h | 6 ++
> kernel/power/hibernate.c | 5 -
> mm/secretmem.c| 15 +++
> 3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/secretmem.h b/include/linux/secretmem.h
> index e617b4afcc62..21c3771e6a56 100644
> --- a/include/linux/secretmem.h
> +++ b/include/linux/secretmem.h
> @@ -30,6 +30,7 @@ static inline bool page_is_secretmem(struct page *page)
> }
>
> bool vma_is_secretmem(struct vm_area_struct *vma);
> +bool secretmem_active(void);
>
> #else
>
> @@ -43,6 +44,11 @@ static inline bool page_is_secretmem(struct page *page)
> return false;
> }
>
> +static inline bool secretmem_active(void)
> +{
> + return false;
> +}
> +
> #endif /* CONFIG_SECRETMEM */
>
> #endif /* _LINUX_SECRETMEM_H */
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index da0b41914177..559acef3fddb 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "power.h"
> @@ -81,7 +82,9 @@ void hibernate_release(void)
>
> bool hibernation_available(void)
> {
> - return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
> + return nohibernate == 0 &&
> + !security_locked_down(LOCKDOWN_HIBERNATION) &&
> + !secretmem_active();
> }
>
> /**
> diff --git a/mm/secretmem.c b/mm/secretmem.c
> index 1ae50089adf1..7c2499e4de22 100644
> --- a/mm/secretmem.c
> +++ b/mm/secretmem.c
> @@ -40,6 +40,13 @@ module_param_named(enable, secretmem_enable, bool, 0400);
> MODULE_PARM_DESC(secretmem_enable,
>"Enable secretmem and memfd_secret(2) system call");
>
> +static atomic_t secretmem_users;
> +
> +bool secretmem_active(void)
> +{
> + return !!atomic_read(&secretmem_users);
> +}
> +
> static vm_fault_t secretmem_fault(struct vm_fault *vmf)
> {
> struct address_space *mapping = vmf->vma->vm_file->f_mapping;
> @@ -94,6 +101,12 @@ static const struct vm_operations_struct secretmem_vm_ops
> = {
> .fault = secretmem_fault,
> };
>
> +static int secretmem_release(struct inode *inode, struct file *file)
> +{
> + atomic_dec(&secretmem_users);
> + return 0;
> +}
> +
> static int secretmem_mmap(struct file *file, struct vm_area_struct *vma)
> {
> unsigned long len = vma->vm_end - vma->vm_start;
> @@ -116,6 +129,7 @@ bool vma_is_secretmem(struct vm_area_struct *vma)
> }
>
> static const struct file_operations secretmem_fops = {
> + .release= secretmem_release,
> .mmap = secretmem_mmap,
> };
>
> @@ -202,6 +216,7 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
> file->f_flags |= O_LARGEFILE;
>
> fd_install(fd, file);
> + atomic_inc(&secretmem_users);
> return fd;
>
> err_put_fd:
> --
> 2.28.0
>
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [PATCH v19 6/8] PM: hibernate: disable when there are active secretmem users
On 13.05.21 20:47, Mike Rapoport wrote:
From: Mike Rapoport
It is unsafe to allow saving of secretmem areas to the hibernation
snapshot as they would be visible after the resume and this essentially
will defeat the purpose of secret memory mappings.
Prevent hibernation whenever there are active secret memory users.
Signed-off-by: Mike Rapoport
Cc: Alexander Viro
Cc: Andy Lutomirski
Cc: Arnd Bergmann
Cc: Borislav Petkov
Cc: Catalin Marinas
Cc: Christopher Lameter
Cc: Dan Williams
Cc: Dave Hansen
Cc: David Hildenbrand
Cc: Elena Reshetova
Cc: Hagen Paul Pfeifer
Cc: "H. Peter Anvin"
Cc: Ingo Molnar
Cc: James Bottomley
Cc: "Kirill A. Shutemov"
Cc: Mark Rutland
Cc: Matthew Wilcox
Cc: Michael Kerrisk
Cc: Palmer Dabbelt
Cc: Palmer Dabbelt
Cc: Paul Walmsley
Cc: Peter Zijlstra
Cc: Rick Edgecombe
Cc: Roman Gushchin
Cc: Shakeel Butt
Cc: Shuah Khan
Cc: Thomas Gleixner
Cc: Tycho Andersen
Cc: Will Deacon
---
include/linux/secretmem.h | 6 ++
kernel/power/hibernate.c | 5 -
mm/secretmem.c| 15 +++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/linux/secretmem.h b/include/linux/secretmem.h
index e617b4afcc62..21c3771e6a56 100644
--- a/include/linux/secretmem.h
+++ b/include/linux/secretmem.h
@@ -30,6 +30,7 @@ static inline bool page_is_secretmem(struct page *page)
}
bool vma_is_secretmem(struct vm_area_struct *vma);
+bool secretmem_active(void);
#else
@@ -43,6 +44,11 @@ static inline bool page_is_secretmem(struct page *page)
return false;
}
+static inline bool secretmem_active(void)
+{
+ return false;
+}
+
#endif /* CONFIG_SECRETMEM */
#endif /* _LINUX_SECRETMEM_H */
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index da0b41914177..559acef3fddb 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include "power.h"
@@ -81,7 +82,9 @@ void hibernate_release(void)
bool hibernation_available(void)
{
- return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
+ return nohibernate == 0 &&
+ !security_locked_down(LOCKDOWN_HIBERNATION) &&
+ !secretmem_active();
}
/**
diff --git a/mm/secretmem.c b/mm/secretmem.c
index 1ae50089adf1..7c2499e4de22 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -40,6 +40,13 @@ module_param_named(enable, secretmem_enable, bool, 0400);
MODULE_PARM_DESC(secretmem_enable,
"Enable secretmem and memfd_secret(2) system call");
+static atomic_t secretmem_users;
+
+bool secretmem_active(void)
+{
+ return !!atomic_read(&secretmem_users);
+}
+
static vm_fault_t secretmem_fault(struct vm_fault *vmf)
{
struct address_space *mapping = vmf->vma->vm_file->f_mapping;
@@ -94,6 +101,12 @@ static const struct vm_operations_struct secretmem_vm_ops =
{
.fault = secretmem_fault,
};
+static int secretmem_release(struct inode *inode, struct file *file)
+{
+ atomic_dec(&secretmem_users);
+ return 0;
+}
+
static int secretmem_mmap(struct file *file, struct vm_area_struct *vma)
{
unsigned long len = vma->vm_end - vma->vm_start;
@@ -116,6 +129,7 @@ bool vma_is_secretmem(struct vm_area_struct *vma)
}
static const struct file_operations secretmem_fops = {
+ .release= secretmem_release,
.mmap = secretmem_mmap,
};
@@ -202,6 +216,7 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
file->f_flags |= O_LARGEFILE;
fd_install(fd, file);
+ atomic_inc(&secretmem_users);
return fd;
err_put_fd:
It looks a bit racy, but I guess we don't really care about these corner
cases.
Acked-by: David Hildenbrand
--
Thanks,
David / dhildenb
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
[PATCH v19 6/8] PM: hibernate: disable when there are active secretmem users
From: Mike Rapoport
It is unsafe to allow saving of secretmem areas to the hibernation
snapshot as they would be visible after the resume and this essentially
will defeat the purpose of secret memory mappings.
Prevent hibernation whenever there are active secret memory users.
Signed-off-by: Mike Rapoport
Cc: Alexander Viro
Cc: Andy Lutomirski
Cc: Arnd Bergmann
Cc: Borislav Petkov
Cc: Catalin Marinas
Cc: Christopher Lameter
Cc: Dan Williams
Cc: Dave Hansen
Cc: David Hildenbrand
Cc: Elena Reshetova
Cc: Hagen Paul Pfeifer
Cc: "H. Peter Anvin"
Cc: Ingo Molnar
Cc: James Bottomley
Cc: "Kirill A. Shutemov"
Cc: Mark Rutland
Cc: Matthew Wilcox
Cc: Michael Kerrisk
Cc: Palmer Dabbelt
Cc: Palmer Dabbelt
Cc: Paul Walmsley
Cc: Peter Zijlstra
Cc: Rick Edgecombe
Cc: Roman Gushchin
Cc: Shakeel Butt
Cc: Shuah Khan
Cc: Thomas Gleixner
Cc: Tycho Andersen
Cc: Will Deacon
---
include/linux/secretmem.h | 6 ++
kernel/power/hibernate.c | 5 -
mm/secretmem.c| 15 +++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/linux/secretmem.h b/include/linux/secretmem.h
index e617b4afcc62..21c3771e6a56 100644
--- a/include/linux/secretmem.h
+++ b/include/linux/secretmem.h
@@ -30,6 +30,7 @@ static inline bool page_is_secretmem(struct page *page)
}
bool vma_is_secretmem(struct vm_area_struct *vma);
+bool secretmem_active(void);
#else
@@ -43,6 +44,11 @@ static inline bool page_is_secretmem(struct page *page)
return false;
}
+static inline bool secretmem_active(void)
+{
+ return false;
+}
+
#endif /* CONFIG_SECRETMEM */
#endif /* _LINUX_SECRETMEM_H */
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index da0b41914177..559acef3fddb 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include "power.h"
@@ -81,7 +82,9 @@ void hibernate_release(void)
bool hibernation_available(void)
{
- return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
+ return nohibernate == 0 &&
+ !security_locked_down(LOCKDOWN_HIBERNATION) &&
+ !secretmem_active();
}
/**
diff --git a/mm/secretmem.c b/mm/secretmem.c
index 1ae50089adf1..7c2499e4de22 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -40,6 +40,13 @@ module_param_named(enable, secretmem_enable, bool, 0400);
MODULE_PARM_DESC(secretmem_enable,
"Enable secretmem and memfd_secret(2) system call");
+static atomic_t secretmem_users;
+
+bool secretmem_active(void)
+{
+ return !!atomic_read(&secretmem_users);
+}
+
static vm_fault_t secretmem_fault(struct vm_fault *vmf)
{
struct address_space *mapping = vmf->vma->vm_file->f_mapping;
@@ -94,6 +101,12 @@ static const struct vm_operations_struct secretmem_vm_ops =
{
.fault = secretmem_fault,
};
+static int secretmem_release(struct inode *inode, struct file *file)
+{
+ atomic_dec(&secretmem_users);
+ return 0;
+}
+
static int secretmem_mmap(struct file *file, struct vm_area_struct *vma)
{
unsigned long len = vma->vm_end - vma->vm_start;
@@ -116,6 +129,7 @@ bool vma_is_secretmem(struct vm_area_struct *vma)
}
static const struct file_operations secretmem_fops = {
+ .release= secretmem_release,
.mmap = secretmem_mmap,
};
@@ -202,6 +216,7 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
file->f_flags |= O_LARGEFILE;
fd_install(fd, file);
+ atomic_inc(&secretmem_users);
return fd;
err_put_fd:
--
2.28.0
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
