| Message ID | 20221202013404.163143-6-jeffxu@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kees Cook |
| Headers | show |
| Series | [v3] mm/memfd: security hook for memfd_create | expand |
On Fri, Dec 02, 2022 at 01:34:03AM +0000, jeffxu@chromium.org wrote: > From: Jeff Xu <jeffxu@chromium.org> > > 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 <jeffxu@chromium.org> > --- > 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 >
On Fri, Dec 2, 2022 at 2:58 PM Kees Cook <keescook@chromium.org> wrote: > > On Fri, Dec 02, 2022 at 01:34:03AM +0000, jeffxu@chromium.org wrote: > > From: Jeff Xu <jeffxu@chromium.org> > > > > 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 <jeffxu@chromium.org> > > --- > > 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
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,