diff mbox

[RFC,v2,3/9] Creation of "check_vmflags" LSM hook

Message ID 1497544976-7856-4-git-send-email-s.mesoraca16@gmail.com
State Superseded
Headers show

Commit Message

Salvatore Mesoraca June 15, 2017, 4:42 p.m. UTC
Creation of a new LSM hook to check if a given configuration of vmflags,
for a new memory allocation request, should be allowed or not.
It's placed in "do_mmap", "do_brk_flags" and "__install_special_mapping".

Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
---
 include/linux/lsm_hooks.h |  7 +++++++
 include/linux/security.h  |  6 ++++++
 mm/mmap.c                 | 13 +++++++++++++
 security/security.c       |  5 +++++
 4 files changed, 31 insertions(+)

Comments

Kees Cook June 27, 2017, 11:05 p.m. UTC | #1
On Thu, Jun 15, 2017 at 9:42 AM, Salvatore Mesoraca
<s.mesoraca16@gmail.com> wrote:
> Creation of a new LSM hook to check if a given configuration of vmflags,
> for a new memory allocation request, should be allowed or not.
> It's placed in "do_mmap", "do_brk_flags" and "__install_special_mapping".

I like this. I think this is something the other LSMs should be
checking too. (Though I wonder if it would be helpful to include the
VMA in the hook, though it does exist yet, so... hmm.)

-Kees

>
> Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
> ---
>  include/linux/lsm_hooks.h |  7 +++++++
>  include/linux/security.h  |  6 ++++++
>  mm/mmap.c                 | 13 +++++++++++++
>  security/security.c       |  5 +++++
>  4 files changed, 31 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index bd274db..33dab16 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -483,6 +483,11 @@
>   *     @reqprot contains the protection requested by the application.
>   *     @prot contains the protection that will be applied by the kernel.
>   *     Return 0 if permission is granted.
> + * @check_vmflags:
> + *     Check if the requested @vmflags are allowed.
> + *     @vmflags contains requested the vmflags.
> + *     Return 0 if the operation is allowed to continue otherwise return
> + *     the appropriate error code.
>   * @file_lock:
>   *     Check permission before performing file locking operations.
>   *     Note: this hook mediates both flock and fcntl style locks.
> @@ -1477,6 +1482,7 @@
>                                 unsigned long prot, unsigned long flags);
>         int (*file_mprotect)(struct vm_area_struct *vma, unsigned long reqprot,
>                                 unsigned long prot);
> +       int (*check_vmflags)(vm_flags_t vmflags);
>         int (*file_lock)(struct file *file, unsigned int cmd);
>         int (*file_fcntl)(struct file *file, unsigned int cmd,
>                                 unsigned long arg);
> @@ -1747,6 +1753,7 @@ struct security_hook_heads {
>         struct list_head mmap_addr;
>         struct list_head mmap_file;
>         struct list_head file_mprotect;
> +       struct list_head check_vmflags;
>         struct list_head file_lock;
>         struct list_head file_fcntl;
>         struct list_head file_set_fowner;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..8701872 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -300,6 +300,7 @@ int security_mmap_file(struct file *file, unsigned long prot,
>  int security_mmap_addr(unsigned long addr);
>  int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
>                            unsigned long prot);
> +int security_check_vmflags(vm_flags_t vmflags);
>  int security_file_lock(struct file *file, unsigned int cmd);
>  int security_file_fcntl(struct file *file, unsigned int cmd, unsigned long arg);
>  void security_file_set_fowner(struct file *file);
> @@ -823,6 +824,11 @@ static inline int security_file_mprotect(struct vm_area_struct *vma,
>         return 0;
>  }
>
> +static inline int security_check_vmflags(vm_flags_t vmflags)
> +{
> +       return 0;
> +}
> +
>  static inline int security_file_lock(struct file *file, unsigned int cmd)
>  {
>         return 0;
> diff --git a/mm/mmap.c b/mm/mmap.c
> index f82741e..132061b 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1311,6 +1311,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
>  {
>         struct mm_struct *mm = current->mm;
>         int pkey = 0;
> +       int error;
>
>         *populate = 0;
>
> @@ -1363,6 +1364,10 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
>         vm_flags |= calc_vm_prot_bits(prot, pkey) | calc_vm_flag_bits(flags) |
>                         mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
>
> +       error = security_check_vmflags(vm_flags);
> +       if (error)
> +               return error;
> +
>         if (flags & MAP_LOCKED)
>                 if (!can_do_mlock())
>                         return -EPERM;
> @@ -2833,6 +2838,10 @@ static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long
>                 return -EINVAL;
>         flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
>
> +       error = security_check_vmflags(flags);
> +       if (error)
> +               return error;
> +
>         error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED);
>         if (offset_in_page(error))
>                 return error;
> @@ -3208,6 +3217,10 @@ static struct vm_area_struct *__install_special_mapping(
>         int ret;
>         struct vm_area_struct *vma;
>
> +       ret = security_check_vmflags(vm_flags);
> +       if (ret)
> +               return ERR_PTR(ret);
> +
>         vma = kmem_cache_zalloc(vm_area_cachep, GFP_KERNEL);
>         if (unlikely(vma == NULL))
>                 return ERR_PTR(-ENOMEM);
> diff --git a/security/security.c b/security/security.c
> index 42c8028..7e45846 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -900,6 +900,11 @@ int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
>         return call_int_hook(file_mprotect, 0, vma, reqprot, prot);
>  }
>
> +int security_check_vmflags(vm_flags_t vmflags)
> +{
> +       return call_int_hook(check_vmflags, 0, vmflags);
> +}
> +
>  int security_file_lock(struct file *file, unsigned int cmd)
>  {
>         return call_int_hook(file_lock, 0, file, cmd);
> --
> 1.9.1
>
Salvatore Mesoraca June 29, 2017, 7:28 p.m. UTC | #2
2017-06-28 1:05 GMT+02:00 Kees Cook <keescook@chromium.org>:
> On Thu, Jun 15, 2017 at 9:42 AM, Salvatore Mesoraca
> <s.mesoraca16@gmail.com> wrote:
>> Creation of a new LSM hook to check if a given configuration of vmflags,
>> for a new memory allocation request, should be allowed or not.
>> It's placed in "do_mmap", "do_brk_flags" and "__install_special_mapping".
>
> I like this. I think this is something the other LSMs should be
> checking too. (Though I wonder if it would be helpful to include the
> VMA in the hook, though it does exist yet, so... hmm.)

For the particular case of my LSM and the type of check it does, the VMA
isn't needed, of course.
Maybe some other LSM can benefit from it, but it depends on what they
want to do with this hook.
Looking forward to feedback from potential future users.
Thank you for your interest.

Salvatore
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index bd274db..33dab16 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -483,6 +483,11 @@ 
  *	@reqprot contains the protection requested by the application.
  *	@prot contains the protection that will be applied by the kernel.
  *	Return 0 if permission is granted.
+ * @check_vmflags:
+ *	Check if the requested @vmflags are allowed.
+ *	@vmflags contains requested the vmflags.
+ *	Return 0 if the operation is allowed to continue otherwise return
+ *	the appropriate error code.
  * @file_lock:
  *	Check permission before performing file locking operations.
  *	Note: this hook mediates both flock and fcntl style locks.
@@ -1477,6 +1482,7 @@ 
 				unsigned long prot, unsigned long flags);
 	int (*file_mprotect)(struct vm_area_struct *vma, unsigned long reqprot,
 				unsigned long prot);
+	int (*check_vmflags)(vm_flags_t vmflags);
 	int (*file_lock)(struct file *file, unsigned int cmd);
 	int (*file_fcntl)(struct file *file, unsigned int cmd,
 				unsigned long arg);
@@ -1747,6 +1753,7 @@  struct security_hook_heads {
 	struct list_head mmap_addr;
 	struct list_head mmap_file;
 	struct list_head file_mprotect;
+	struct list_head check_vmflags;
 	struct list_head file_lock;
 	struct list_head file_fcntl;
 	struct list_head file_set_fowner;
diff --git a/include/linux/security.h b/include/linux/security.h
index af675b5..8701872 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -300,6 +300,7 @@  int security_mmap_file(struct file *file, unsigned long prot,
 int security_mmap_addr(unsigned long addr);
 int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
 			   unsigned long prot);
+int security_check_vmflags(vm_flags_t vmflags);
 int security_file_lock(struct file *file, unsigned int cmd);
 int security_file_fcntl(struct file *file, unsigned int cmd, unsigned long arg);
 void security_file_set_fowner(struct file *file);
@@ -823,6 +824,11 @@  static inline int security_file_mprotect(struct vm_area_struct *vma,
 	return 0;
 }
 
+static inline int security_check_vmflags(vm_flags_t vmflags)
+{
+	return 0;
+}
+
 static inline int security_file_lock(struct file *file, unsigned int cmd)
 {
 	return 0;
diff --git a/mm/mmap.c b/mm/mmap.c
index f82741e..132061b 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1311,6 +1311,7 @@  unsigned long do_mmap(struct file *file, unsigned long addr,
 {
 	struct mm_struct *mm = current->mm;
 	int pkey = 0;
+	int error;
 
 	*populate = 0;
 
@@ -1363,6 +1364,10 @@  unsigned long do_mmap(struct file *file, unsigned long addr,
 	vm_flags |= calc_vm_prot_bits(prot, pkey) | calc_vm_flag_bits(flags) |
 			mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
 
+	error = security_check_vmflags(vm_flags);
+	if (error)
+		return error;
+
 	if (flags & MAP_LOCKED)
 		if (!can_do_mlock())
 			return -EPERM;
@@ -2833,6 +2838,10 @@  static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long
 		return -EINVAL;
 	flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
 
+	error = security_check_vmflags(flags);
+	if (error)
+		return error;
+
 	error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED);
 	if (offset_in_page(error))
 		return error;
@@ -3208,6 +3217,10 @@  static struct vm_area_struct *__install_special_mapping(
 	int ret;
 	struct vm_area_struct *vma;
 
+	ret = security_check_vmflags(vm_flags);
+	if (ret)
+		return ERR_PTR(ret);
+
 	vma = kmem_cache_zalloc(vm_area_cachep, GFP_KERNEL);
 	if (unlikely(vma == NULL))
 		return ERR_PTR(-ENOMEM);
diff --git a/security/security.c b/security/security.c
index 42c8028..7e45846 100644
--- a/security/security.c
+++ b/security/security.c
@@ -900,6 +900,11 @@  int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
 	return call_int_hook(file_mprotect, 0, vma, reqprot, prot);
 }
 
+int security_check_vmflags(vm_flags_t vmflags)
+{
+	return call_int_hook(check_vmflags, 0, vmflags);
+}
+
 int security_file_lock(struct file *file, unsigned int cmd)
 {
 	return call_int_hook(file_lock, 0, file, cmd);