From: Jeff Xu <[email protected]>
Since Linux introduced the memfd feature, memfd have always had their
execute bit set, and the memfd_create() syscall doesn't allow setting
it differently.
However, in a secure by default system, such as ChromeOS, (where all
executables should come from the rootfs, which is protected by Verified
boot), this executable nature of memfd opens a door for NoExec bypass
and enables “confused deputy attack”. E.g, in VRP bug [1]: cros_vm
process created a memfd to share the content with an external process,
however the memfd is overwritten and used for executing arbitrary code
and root escalation. [2] lists more VRP in this kind.
On the other hand, executable memfd has its legit use, runc uses memfd’s
seal and executable feature to copy the contents of the binary then
execute them, for such system, we need a solution to differentiate runc's
use of executable memfds and an attacker's [3].
To address those above, this set of patches add following:
1> Let memfd_create() set X bit at creation time.
2> Let memfd to be sealed for modifying X bit.
3> A new pid namespace sysctl: vm.memfd_noexec to control the behavior of
X bit.For example, if a container has vm.memfd_noexec=2, then
memfd_create() without MFD_NOEXEC_SEAL will be rejected.
4> A new security hook in memfd_create(). This make it possible to a new
LSM, which rejects or allows executable memfd based on its security policy.
This is V6 version of patch: see [4] [5] [6] [7] for previous versions.
[1] https://crbug.com/1305411
[2] https://bugs.chromium.org/p/chromium/issues/list?q=type%3Dbug-security%20memfd%20escalation&can=1
[3] https://lwn.net/Articles/781013/
[4] https://lwn.net/Articles/890096/
[5] https://lore.kernel.org/lkml/[email protected]/
[6] https://lore.kernel.org/lkml/[email protected]/
[7] https://lore.kernel.org/lkml/[email protected]/
Daniel Verkamp (2):
mm/memfd: add F_SEAL_EXEC
selftests/memfd: add tests for F_SEAL_EXEC
Jeff Xu (4):
mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC
mm/memfd: Add write seals when apply SEAL_EXEC to executable memfd
selftests/memfd: add tests for MFD_NOEXEC_SEAL MFD_EXEC
mm/memfd: security hook for memfd_create
include/linux/lsm_hook_defs.h | 1 +
include/linux/lsm_hooks.h | 4 +
include/linux/pid_namespace.h | 19 ++
include/linux/security.h | 6 +
include/uapi/linux/fcntl.h | 1 +
include/uapi/linux/memfd.h | 4 +
kernel/pid_namespace.c | 5 +
kernel/pid_sysctl.h | 59 ++++
mm/memfd.c | 61 +++-
mm/shmem.c | 6 +
security/security.c | 13 +
tools/testing/selftests/memfd/fuse_test.c | 1 +
tools/testing/selftests/memfd/memfd_test.c | 348 ++++++++++++++++++++-
13 files changed, 525 insertions(+), 3 deletions(-)
create mode 100644 kernel/pid_sysctl.h
base-commit: eb7081409f94a9a8608593d0fb63a1aa3d6f95d8
--
2.39.0.rc0.267.gcb52ba06e7-goog
From: Jeff Xu <[email protected]>
The new security_memfd_create allows lsm to check flags of
memfd_create.
The security by default system (such as chromeos) can use this
to implement system wide lsm to allow only non-executable memfd
being created.
Signed-off-by: Jeff Xu <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
include/linux/lsm_hook_defs.h | 1 +
include/linux/lsm_hooks.h | 4 ++++
include/linux/security.h | 6 ++++++
mm/memfd.c | 5 +++++
security/security.c | 13 +++++++++++++
5 files changed, 29 insertions(+)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index ec119da1d89b..fd40840927c8 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -164,6 +164,7 @@ LSM_HOOK(int, 0, file_alloc_security, struct file *file)
LSM_HOOK(void, LSM_RET_VOID, file_free_security, struct file *file)
LSM_HOOK(int, 0, file_ioctl, struct file *file, unsigned int cmd,
unsigned long arg)
+LSM_HOOK(int, 0, memfd_create, char *name, unsigned int flags)
LSM_HOOK(int, 0, mmap_addr, unsigned long addr)
LSM_HOOK(int, 0, mmap_file, struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4ec80b96c22e..5a18a6552278 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -543,6 +543,10 @@
* simple integer value. When @arg represents a user space pointer, it
* should never be used by the security module.
* Return 0 if permission is granted.
+ * @memfd_create:
+ * @name is the name of memfd file.
+ * @flags is the flags used in memfd_create.
+ * Return 0 if permission is granted.
* @mmap_addr :
* Check permissions for a mmap operation at @addr.
* @addr contains virtual address that will be used for the operation.
diff --git a/include/linux/security.h b/include/linux/security.h
index ca1b7109c0db..5b87a780822a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -384,6 +384,7 @@ int security_file_permission(struct file *file, int mask);
int security_file_alloc(struct file *file);
void security_file_free(struct file *file);
int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+int security_memfd_create(char *name, unsigned int flags);
int security_mmap_file(struct file *file, unsigned long prot,
unsigned long flags);
int security_mmap_addr(unsigned long addr);
@@ -963,6 +964,11 @@ static inline int security_file_ioctl(struct file *file, unsigned int cmd,
return 0;
}
+static inline int security_memfd_create(char *name, unsigned int flags)
+{
+ return 0;
+}
+
static inline int security_mmap_file(struct file *file, unsigned long prot,
unsigned long flags)
{
diff --git a/mm/memfd.c b/mm/memfd.c
index 92f0a5765f7c..f04ed5f0474f 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -356,6 +356,11 @@ SYSCALL_DEFINE2(memfd_create,
goto err_name;
}
+ /* security hook for memfd_create */
+ error = security_memfd_create(name, flags);
+ if (error)
+ return error;
+
if (flags & MFD_HUGETLB) {
file = hugetlb_file_setup(name, 0, VM_NORESERVE,
HUGETLB_ANONHUGE_INODE,
diff --git a/security/security.c b/security/security.c
index 79d82cb6e469..5c018e080923 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1010,6 +1010,19 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
}
EXPORT_SYMBOL(security_sb_clone_mnt_opts);
+int security_add_mnt_opt(const char *option, const char *val, int len,
+ void **mnt_opts)
+{
+ return call_int_hook(sb_add_mnt_opt, -EINVAL,
+ option, val, len, mnt_opts);
+}
+EXPORT_SYMBOL(security_add_mnt_opt);
+
+int security_memfd_create(char *name, unsigned int flags)
+{
+ return call_int_hook(memfd_create, 0, name, flags);
+}
+
int security_move_mount(const struct path *from_path, const struct path *to_path)
{
return call_int_hook(move_mount, 0, from_path, to_path);
--
2.39.0.rc0.267.gcb52ba06e7-goog
On Wed, Dec 07, 2022 at 03:49:33PM +0000, [email protected] wrote:
> This is V6 version of patch: see [4] [5] [6] [7] for previous versions.
When sending a new version, can you include an overview of what changed
between this version and the prior version? This helps reviewers who are
following along, so it's easier to focus our attention on the
differences. Also, it's helpful to version the links:
> [4] https://lwn.net/Articles/890096/
> [5] https://lore.kernel.org/lkml/[email protected]/
> [6] https://lore.kernel.org/lkml/[email protected]/
> [7] https://lore.kernel.org/lkml/[email protected]/
e.g.:
v6:
- moved foo to bar
- improve comments for baz
v5: https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v2: ...etc
-Kees
--
Kees Cook
On Wed, Dec 07, 2022 at 03:49:39PM +0000, [email protected] wrote:
> From: Jeff Xu <[email protected]>
>
> The new security_memfd_create allows lsm to check flags of
> memfd_create.
>
> The security by default system (such as chromeos) can use this
> to implement system wide lsm to allow only non-executable memfd
> being created.
>
> Signed-off-by: Jeff Xu <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> ---
> include/linux/lsm_hook_defs.h | 1 +
> include/linux/lsm_hooks.h | 4 ++++
> include/linux/security.h | 6 ++++++
> mm/memfd.c | 5 +++++
> security/security.c | 13 +++++++++++++
> 5 files changed, 29 insertions(+)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index ec119da1d89b..fd40840927c8 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -164,6 +164,7 @@ LSM_HOOK(int, 0, file_alloc_security, struct file *file)
> LSM_HOOK(void, LSM_RET_VOID, file_free_security, struct file *file)
> LSM_HOOK(int, 0, file_ioctl, struct file *file, unsigned int cmd,
> unsigned long arg)
> +LSM_HOOK(int, 0, memfd_create, char *name, unsigned int flags)
> LSM_HOOK(int, 0, mmap_addr, unsigned long addr)
> LSM_HOOK(int, 0, mmap_file, struct file *file, unsigned long reqprot,
> unsigned long prot, unsigned long flags)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 4ec80b96c22e..5a18a6552278 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -543,6 +543,10 @@
> * simple integer value. When @arg represents a user space pointer, it
> * should never be used by the security module.
> * Return 0 if permission is granted.
> + * @memfd_create:
> + * @name is the name of memfd file.
> + * @flags is the flags used in memfd_create.
> + * Return 0 if permission is granted.
> * @mmap_addr :
> * Check permissions for a mmap operation at @addr.
> * @addr contains virtual address that will be used for the operation.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index ca1b7109c0db..5b87a780822a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -384,6 +384,7 @@ int security_file_permission(struct file *file, int mask);
> int security_file_alloc(struct file *file);
> void security_file_free(struct file *file);
> int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
> +int security_memfd_create(char *name, unsigned int flags);
> int security_mmap_file(struct file *file, unsigned long prot,
> unsigned long flags);
> int security_mmap_addr(unsigned long addr);
> @@ -963,6 +964,11 @@ static inline int security_file_ioctl(struct file *file, unsigned int cmd,
> return 0;
> }
>
> +static inline int security_memfd_create(char *name, unsigned int flags)
> +{
> + return 0;
> +}
> +
> static inline int security_mmap_file(struct file *file, unsigned long prot,
> unsigned long flags)
> {
> diff --git a/mm/memfd.c b/mm/memfd.c
> index 92f0a5765f7c..f04ed5f0474f 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -356,6 +356,11 @@ SYSCALL_DEFINE2(memfd_create,
> goto err_name;
> }
>
> + /* security hook for memfd_create */
> + error = security_memfd_create(name, flags);
> + if (error)
> + return error;
> +
> if (flags & MFD_HUGETLB) {
> file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> HUGETLB_ANONHUGE_INODE,
> diff --git a/security/security.c b/security/security.c
> index 79d82cb6e469..5c018e080923 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1010,6 +1010,19 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> }
> EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>
> +int security_add_mnt_opt(const char *option, const char *val, int len,
> + void **mnt_opts)
> +{
> + return call_int_hook(sb_add_mnt_opt, -EINVAL,
> + option, val, len, mnt_opts);
> +}
> +EXPORT_SYMBOL(security_add_mnt_opt);
I think security_add_mnt_opt() isn't supposed to be in here. :)
> +
> +int security_memfd_create(char *name, unsigned int flags)
> +{
> + return call_int_hook(memfd_create, 0, name, flags);
> +}
> +
> int security_move_mount(const struct path *from_path, const struct path *to_path)
> {
> return call_int_hook(move_mount, 0, from_path, to_path);
> --
> 2.39.0.rc0.267.gcb52ba06e7-goog
>
Otherwise looks good.
Thanks!
-Kees
--
Kees Cook
On Wed, Dec 07, 2022 at 03:49:39PM +0000, [email protected] wrote:
> From: Jeff Xu <[email protected]>
>
> The new security_memfd_create allows lsm to check flags of
> memfd_create.
>
> The security by default system (such as chromeos) can use this
> to implement system wide lsm to allow only non-executable memfd
> being created.
>
> Signed-off-by: Jeff Xu <[email protected]>
> Reported-by: kernel test robot <[email protected]>
Oh, btw, please CC [email protected] when adding new
hooks. (I've added the CC here.)
-Kees
> ---
> include/linux/lsm_hook_defs.h | 1 +
> include/linux/lsm_hooks.h | 4 ++++
> include/linux/security.h | 6 ++++++
> mm/memfd.c | 5 +++++
> security/security.c | 13 +++++++++++++
> 5 files changed, 29 insertions(+)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index ec119da1d89b..fd40840927c8 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -164,6 +164,7 @@ LSM_HOOK(int, 0, file_alloc_security, struct file *file)
> LSM_HOOK(void, LSM_RET_VOID, file_free_security, struct file *file)
> LSM_HOOK(int, 0, file_ioctl, struct file *file, unsigned int cmd,
> unsigned long arg)
> +LSM_HOOK(int, 0, memfd_create, char *name, unsigned int flags)
> LSM_HOOK(int, 0, mmap_addr, unsigned long addr)
> LSM_HOOK(int, 0, mmap_file, struct file *file, unsigned long reqprot,
> unsigned long prot, unsigned long flags)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 4ec80b96c22e..5a18a6552278 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -543,6 +543,10 @@
> * simple integer value. When @arg represents a user space pointer, it
> * should never be used by the security module.
> * Return 0 if permission is granted.
> + * @memfd_create:
> + * @name is the name of memfd file.
> + * @flags is the flags used in memfd_create.
> + * Return 0 if permission is granted.
> * @mmap_addr :
> * Check permissions for a mmap operation at @addr.
> * @addr contains virtual address that will be used for the operation.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index ca1b7109c0db..5b87a780822a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -384,6 +384,7 @@ int security_file_permission(struct file *file, int mask);
> int security_file_alloc(struct file *file);
> void security_file_free(struct file *file);
> int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
> +int security_memfd_create(char *name, unsigned int flags);
> int security_mmap_file(struct file *file, unsigned long prot,
> unsigned long flags);
> int security_mmap_addr(unsigned long addr);
> @@ -963,6 +964,11 @@ static inline int security_file_ioctl(struct file *file, unsigned int cmd,
> return 0;
> }
>
> +static inline int security_memfd_create(char *name, unsigned int flags)
> +{
> + return 0;
> +}
> +
> static inline int security_mmap_file(struct file *file, unsigned long prot,
> unsigned long flags)
> {
> diff --git a/mm/memfd.c b/mm/memfd.c
> index 92f0a5765f7c..f04ed5f0474f 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -356,6 +356,11 @@ SYSCALL_DEFINE2(memfd_create,
> goto err_name;
> }
>
> + /* security hook for memfd_create */
> + error = security_memfd_create(name, flags);
> + if (error)
> + return error;
> +
> if (flags & MFD_HUGETLB) {
> file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> HUGETLB_ANONHUGE_INODE,
> diff --git a/security/security.c b/security/security.c
> index 79d82cb6e469..5c018e080923 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1010,6 +1010,19 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> }
> EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>
> +int security_add_mnt_opt(const char *option, const char *val, int len,
> + void **mnt_opts)
> +{
> + return call_int_hook(sb_add_mnt_opt, -EINVAL,
> + option, val, len, mnt_opts);
> +}
> +EXPORT_SYMBOL(security_add_mnt_opt);
> +
> +int security_memfd_create(char *name, unsigned int flags)
> +{
> + return call_int_hook(memfd_create, 0, name, flags);
> +}
> +
> int security_move_mount(const struct path *from_path, const struct path *to_path)
> {
> return call_int_hook(move_mount, 0, from_path, to_path);
> --
> 2.39.0.rc0.267.gcb52ba06e7-goog
>
--
Kees Cook
On Thu, Dec 8, 2022 at 8:13 AM Kees Cook <[email protected]> wrote:
>
> On Wed, Dec 07, 2022 at 03:49:33PM +0000, [email protected] wrote:
> > This is V6 version of patch: see [4] [5] [6] [7] for previous versions.
>
> When sending a new version, can you include an overview of what changed
> between this version and the prior version? This helps reviewers who are
> following along, so it's easier to focus our attention on the
> differences. Also, it's helpful to version the links:
>
> > [4] https://lwn.net/Articles/890096/
> > [5] https://lore.kernel.org/lkml/[email protected]/
> > [6] https://lore.kernel.org/lkml/[email protected]/
> > [7] https://lore.kernel.org/lkml/[email protected]/
>
> e.g.:
>
> v6:
> - moved foo to bar
> - improve comments for baz
> v5: https://lore.kernel.org/lkml/[email protected]/
> v3: https://lore.kernel.org/lkml/[email protected]/
> v2: ...etc
>
Will do!
Much appreciated for helping me through the process of my first patch
in the kernel.
Jeff
> -Kees
>
> --
> Kees Cook
On Thu, Dec 08, 2022 at 10:33:19AM -0800, Jeff Xu wrote:
> On Thu, Dec 8, 2022 at 8:13 AM Kees Cook <[email protected]> wrote:
> >
> > On Wed, Dec 07, 2022 at 03:49:33PM +0000, [email protected] wrote:
> > > This is V6 version of patch: see [4] [5] [6] [7] for previous versions.
> >
> > When sending a new version, can you include an overview of what changed
> > between this version and the prior version? This helps reviewers who are
> > following along, so it's easier to focus our attention on the
> > differences. Also, it's helpful to version the links:
> >
> > > [4] https://lwn.net/Articles/890096/
> > > [5] https://lore.kernel.org/lkml/[email protected]/
> > > [6] https://lore.kernel.org/lkml/[email protected]/
> > > [7] https://lore.kernel.org/lkml/[email protected]/
> >
> > e.g.:
> >
> > v6:
> > - moved foo to bar
> > - improve comments for baz
> > v5: https://lore.kernel.org/lkml/[email protected]/
> > v3: https://lore.kernel.org/lkml/[email protected]/
> > v2: ...etc
> >
> Will do!
> Much appreciated for helping me through the process of my first patch
> in the kernel.
Happy to help! I'm excited to see this gap in memfd security closed. :)
--
Kees Cook