2022-12-02 01:46:15

by Jeff Xu

[permalink] [raw]
Subject: [PATCH v3] mm/memfd: MFD_NOEXEC_SEAL and MFD_EXEC

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 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 V3 version of patch: see [4] [5] 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]/

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
selftests/memfd: add tests for MFD_NOEXEC_SEAL MFD_EXEC
mm/memfd: security hook for memfd_create
mm/memfd: Add write seals when apply SEAL_EXEC to executable memfd

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 | 47 ++++
mm/memfd.c | 54 +++-
mm/shmem.c | 6 +
tools/testing/selftests/memfd/fuse_test.c | 1 +
tools/testing/selftests/memfd/memfd_test.c | 305 ++++++++++++++++++++-
11 files changed, 445 insertions(+), 3 deletions(-)


base-commit: eb7081409f94a9a8608593d0fb63a1aa3d6f95d8
--
2.39.0.rc0.267.gcb52ba06e7-goog


2022-12-02 01:47:01

by Jeff Xu

[permalink] [raw]
Subject: [PATCH v3] selftests/memfd: add tests for MFD_NOEXEC_SEAL MFD_EXEC

From: Jeff Xu <[email protected]>

Tests to verify MFD_NOEXEC, MFD_EXEC and vm.memfd_noexec sysctl.

Co-developed-by: Daniel Verkamp <[email protected]>
Signed-off-by: Daniel Verkamp <[email protected]>
Signed-off-by: Jeff Xu <[email protected]>
---
tools/testing/selftests/memfd/fuse_test.c | 1 +
tools/testing/selftests/memfd/memfd_test.c | 161 ++++++++++++++++++++-
2 files changed, 157 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/memfd/fuse_test.c b/tools/testing/selftests/memfd/fuse_test.c
index be675002f918..93798c8c5d54 100644
--- a/tools/testing/selftests/memfd/fuse_test.c
+++ b/tools/testing/selftests/memfd/fuse_test.c
@@ -22,6 +22,7 @@
#include <linux/falloc.h>
#include <fcntl.h>
#include <linux/memfd.h>
+#include <linux/types.h>
#include <sched.h>
#include <stdio.h>
#include <stdlib.h>
diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
index 1d7e7b36bbdd..775c9e6c061e 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -36,6 +36,10 @@
#define MAX_PATH 256
#endif

+#ifndef MFD_NOEXEC_SEAL
+#define MFD_NOEXEC_SEAL 0x0008U
+#endif
+
/*
* Default is not to test hugetlbfs
*/
@@ -86,6 +90,21 @@ static int mfd_assert_new(const char *name, loff_t sz, unsigned int flags)
return fd;
}

+static void sysctl_assert_write(const char *val)
+{
+ int fd = open("/proc/sys/vm/memfd_noexec", O_WRONLY | O_CLOEXEC);
+
+ if (fd < 0) {
+ printf("open sysctl failed\n");
+ abort();
+ }
+
+ if (write(fd, val, strlen(val)) < 0) {
+ printf("write sysctl failed\n");
+ abort();
+ }
+}
+
static int mfd_assert_reopen_fd(int fd_in)
{
int fd;
@@ -764,6 +783,9 @@ static void test_create(void)
mfd_fail_new("", ~0);
mfd_fail_new("", 0x80000000U);

+ /* verify EXEC and NOEXEC_SEAL can't both be set */
+ mfd_fail_new("", MFD_EXEC | MFD_NOEXEC_SEAL);
+
/* verify MFD_CLOEXEC is allowed */
fd = mfd_assert_new("", 0, MFD_CLOEXEC);
close(fd);
@@ -975,9 +997,10 @@ static void test_seal_resize(void)

/*
* Test SEAL_EXEC
- * Test that chmod() cannot change x bits after sealing
+ * Test fd is created with exec and allow sealing.
+ * chmod() cannot change x bits after sealing.
*/
-static void test_seal_exec(void)
+static void test_exec_seal(void)
{
int fd;

@@ -985,10 +1008,9 @@ static void test_seal_exec(void)

fd = mfd_assert_new("kern_memfd_seal_exec",
mfd_def_size,
- MFD_CLOEXEC | MFD_ALLOW_SEALING);
+ MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_EXEC);

mfd_assert_mode(fd, 0777);
-
mfd_assert_chmod(fd, 0644);

mfd_assert_has_seals(fd, 0);
@@ -1006,6 +1028,131 @@ static void test_seal_exec(void)
close(fd);
}

+/*
+ * Test EXEC_NO_SEAL
+ * Test fd is created with exec and not allow sealing.
+ */
+static void test_exec_no_seal(void)
+{
+ int fd;
+
+ printf("%s EXEC_NO_SEAL\n", memfd_str);
+
+ /* Create with EXEC but without ALLOW_SEALING */
+ fd = mfd_assert_new("kern_memfd_exec_no_sealing",
+ mfd_def_size,
+ MFD_CLOEXEC | MFD_EXEC);
+ mfd_assert_mode(fd, 0777);
+ mfd_assert_has_seals(fd, F_SEAL_SEAL);
+ mfd_assert_chmod(fd, 0666);
+ close(fd);
+}
+
+/*
+ * Test memfd_create with MFD_NOEXEC flag
+ */
+static void test_noexec_seal(void)
+{
+ int fd;
+
+ printf("%s NOEXEC_SEAL\n", memfd_str);
+
+ /* Create with NOEXEC and ALLOW_SEALING */
+ fd = mfd_assert_new("kern_memfd_noexec",
+ mfd_def_size,
+ MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_NOEXEC_SEAL);
+ mfd_assert_mode(fd, 0666);
+ mfd_assert_has_seals(fd, F_SEAL_EXEC);
+ mfd_fail_chmod(fd, 0777);
+ close(fd);
+
+ /* Create with NOEXEC but without ALLOW_SEALING */
+ fd = mfd_assert_new("kern_memfd_noexec",
+ mfd_def_size,
+ MFD_CLOEXEC | MFD_NOEXEC_SEAL);
+ mfd_assert_mode(fd, 0666);
+ mfd_assert_has_seals(fd, F_SEAL_EXEC);
+ mfd_fail_chmod(fd, 0777);
+ close(fd);
+}
+
+static void test_sysctl_child(void)
+{
+ int fd, pid, ret;
+
+ printf("%s sysctl 0\n", memfd_str);
+ sysctl_assert_write("0");
+ fd = mfd_assert_new("kern_memfd_sysctl_0",
+ mfd_def_size,
+ MFD_CLOEXEC | MFD_ALLOW_SEALING);
+
+ mfd_assert_mode(fd, 0777);
+ mfd_assert_has_seals(fd, 0);
+ mfd_assert_chmod(fd, 0644);
+ close(fd);
+
+ printf("%s sysctl 1\n", memfd_str);
+ sysctl_assert_write("1");
+ fd = mfd_assert_new("kern_memfd_sysctl_1",
+ mfd_def_size,
+ MFD_CLOEXEC | MFD_ALLOW_SEALING);
+
+ mfd_assert_mode(fd, 0666);
+ mfd_assert_has_seals(fd, F_SEAL_EXEC);
+ mfd_fail_chmod(fd, 0777);
+ close(fd);
+
+ printf("%s sysctl 2\n", memfd_str);
+ sysctl_assert_write("2");
+ mfd_fail_new("kern_memfd_sysctl_2",
+ MFD_CLOEXEC | MFD_ALLOW_SEALING);
+}
+
+static int newpid_thread_fn(void *arg)
+{
+ test_sysctl_child();
+ return 0;
+}
+
+static pid_t spawn_newpid_thread(unsigned int flags)
+{
+ uint8_t *stack;
+ pid_t pid;
+
+ stack = malloc(STACK_SIZE);
+ if (!stack) {
+ printf("malloc(STACK_SIZE) failed: %m\n");
+ abort();
+ }
+
+ pid = clone(newpid_thread_fn,
+ stack + STACK_SIZE,
+ SIGCHLD | flags,
+ NULL);
+ if (pid < 0) {
+ printf("clone() failed: %m\n");
+ abort();
+ }
+
+ return pid;
+}
+
+static void join_newpid_thread(pid_t pid)
+{
+ waitpid(pid, NULL, 0);
+}
+
+/*
+ * Test sysctl
+ * A very basic sealing test to see whether setting/retrieving seals works.
+ */
+static void test_sysctl(void)
+{
+ int pid = spawn_newpid_thread(CLONE_NEWPID);
+
+ join_newpid_thread(pid);
+}
+
/*
* Test sharing via dup()
* Test that seals are shared between dupped FDs and they're all equal.
@@ -1179,13 +1326,15 @@ int main(int argc, char **argv)

test_create();
test_basic();
+ test_exec_seal();
+ test_exec_no_seal();
+ test_noexec_seal();

test_seal_write();
test_seal_future_write();
test_seal_shrink();
test_seal_grow();
test_seal_resize();
- test_seal_exec();

test_share_dup("SHARE-DUP", "");
test_share_mmap("SHARE-MMAP", "");
@@ -1201,6 +1350,8 @@ int main(int argc, char **argv)
test_share_fork("SHARE-FORK", SHARED_FT_STR);
join_idle_thread(pid);

+ test_sysctl();
+
printf("memfd: DONE\n");

return 0;
--
2.39.0.rc0.267.gcb52ba06e7-goog

2022-12-02 02:08:24

by Jeff Xu

[permalink] [raw]
Subject: [PATCH v3] mm/memfd: security hook for memfd_create

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]>
---
include/linux/lsm_hook_defs.h | 1 +
include/linux/lsm_hooks.h | 4 ++++
include/linux/security.h | 6 ++++++
mm/memfd.c | 5 +++++
4 files changed, 16 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 69e897dea6d5..96dcfbfed09e 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -346,6 +346,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,
--
2.39.0.rc0.267.gcb52ba06e7-goog

2022-12-02 10:37:45

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3] mm/memfd: security hook for memfd_create

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]

url: https://github.com/intel-lab-lkp/linux/commits/jeffxu-chromium-org/mm-memfd-security-hook-for-memfd_create/20221202-094044
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20221202013404.163143-6-jeffxu%40google.com
patch subject: [PATCH v3] mm/memfd: security hook for memfd_create
config: x86_64-defconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/63daa2490ad1a865f02ff504c8c3fcd6fd72c0c3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review jeffxu-chromium-org/mm-memfd-security-hook-for-memfd_create/20221202-094044
git checkout 63daa2490ad1a865f02ff504c8c3fcd6fd72c0c3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

ld: mm/memfd.o: in function `__do_sys_memfd_create':
>> memfd.c:(.text+0xe4): undefined reference to `security_memfd_create'

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (1.40 kB)
config (179.87 kB)
Download all attachments

2022-12-02 13:18:57

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3] mm/memfd: security hook for memfd_create

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]

url: https://github.com/intel-lab-lkp/linux/commits/jeffxu-chromium-org/mm-memfd-security-hook-for-memfd_create/20221202-094044
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20221202013404.163143-6-jeffxu%40google.com
patch subject: [PATCH v3] mm/memfd: security hook for memfd_create
config: i386-randconfig-a001
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/63daa2490ad1a865f02ff504c8c3fcd6fd72c0c3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review jeffxu-chromium-org/mm-memfd-security-hook-for-memfd_create/20221202-094044
git checkout 63daa2490ad1a865f02ff504c8c3fcd6fd72c0c3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

ld: mm/memfd.o: in function `__do_sys_memfd_create':
>> mm/memfd.c:316: undefined reference to `security_memfd_create'


vim +316 mm/memfd.c

265
266 SYSCALL_DEFINE2(memfd_create,
267 const char __user *, uname,
268 unsigned int, flags)
269 {
270 unsigned int *file_seals;
271 struct file *file;
272 int fd, error;
273 char *name;
274 long len;
275
276 if (!(flags & MFD_HUGETLB)) {
277 if (flags & ~(unsigned int)MFD_ALL_FLAGS)
278 return -EINVAL;
279 } else {
280 /* Allow huge page size encoding in flags. */
281 if (flags & ~(unsigned int)(MFD_ALL_FLAGS |
282 (MFD_HUGE_MASK << MFD_HUGE_SHIFT)))
283 return -EINVAL;
284 }
285
286 /* length includes terminating zero */
287 len = strnlen_user(uname, MFD_NAME_MAX_LEN + 1);
288 if (len <= 0)
289 return -EFAULT;
290 if (len > MFD_NAME_MAX_LEN + 1)
291 return -EINVAL;
292
293 name = kmalloc(len + MFD_NAME_PREFIX_LEN, GFP_KERNEL);
294 if (!name)
295 return -ENOMEM;
296
297 strcpy(name, MFD_NAME_PREFIX);
298 if (copy_from_user(&name[MFD_NAME_PREFIX_LEN], uname, len)) {
299 error = -EFAULT;
300 goto err_name;
301 }
302
303 /* terminating-zero may have changed after strnlen_user() returned */
304 if (name[len + MFD_NAME_PREFIX_LEN - 1]) {
305 error = -EFAULT;
306 goto err_name;
307 }
308
309 fd = get_unused_fd_flags((flags & MFD_CLOEXEC) ? O_CLOEXEC : 0);
310 if (fd < 0) {
311 error = fd;
312 goto err_name;
313 }
314
315 /* security hook for memfd_create */
> 316 error = security_memfd_create(name, flags);

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.99 kB)
config (155.71 kB)
Download all attachments

2022-12-02 22:50:04

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3] mm/memfd: MFD_NOEXEC_SEAL and MFD_EXEC

On Fri, Dec 02, 2022 at 01:33:58AM +0000, [email protected] wrote:
> From: Jeff Xu <[email protected]>

Hi! Thanks for this update! For future versions, please also use "-n"
with "git format-patch" so the patches are numbered, otherwise it's more
difficult to figure out what order they should be applied in, etc.

--
Kees Cook

2022-12-02 23:23:23

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3] mm/memfd: security hook for memfd_create

On Fri, Dec 02, 2022 at 01:34:03AM +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]>
> ---
> include/linux/lsm_hook_defs.h | 1 +
> include/linux/lsm_hooks.h | 4 ++++
> include/linux/security.h | 6 ++++++
> mm/memfd.c | 5 +++++
> 4 files changed, 16 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;
> +}

I think this is missing the security/security.c changes for the
non-inline version?

-Kees

> +
> 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 69e897dea6d5..96dcfbfed09e 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -346,6 +346,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,
> --
> 2.39.0.rc0.267.gcb52ba06e7-goog
>

--
Kees Cook

2022-12-02 23:51:43

by Jeff Xu

[permalink] [raw]
Subject: Re: [PATCH v3] mm/memfd: security hook for memfd_create

On Fri, Dec 2, 2022 at 2:58 PM Kees Cook <[email protected]> wrote:
>
> On Fri, Dec 02, 2022 at 01:34:03AM +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]>
> > ---
> > include/linux/lsm_hook_defs.h | 1 +
> > include/linux/lsm_hooks.h | 4 ++++
> > include/linux/security.h | 6 ++++++
> > mm/memfd.c | 5 +++++
> > 4 files changed, 16 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;
> > +}
>
> I think this is missing the security/security.c changes for the
> non-inline version?
>
Yes. I will add that in V4.

> -Kees
>
> > +
> > 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 69e897dea6d5..96dcfbfed09e 100644
> > --- a/mm/memfd.c
> > +++ b/mm/memfd.c
> > @@ -346,6 +346,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,
> > --
> > 2.39.0.rc0.267.gcb52ba06e7-goog
> >
>
> --
> Kees Cook