Re: [PATCH] Protection for exploiting null dereference using mmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2007-06-06 at 02:30 -0400, Eric Paris wrote:
> On Tue, 2007-06-05 at 17:16 -0400, Alan Cox wrote:
> > On Tue, Jun 05, 2007 at 05:00:51PM -0400, James Morris wrote:
> > > This should be an unsigned long.
> > > 
> > > I wonder if the default should be for this value to be zero (i.e. preserve 
> > > existing behavior).  It could break binaries, albeit potentially insecure 
> > 
> > Agreed - DOSemu type apps and lrmi need to map at zero for vm86
> 
> And so it shall be!
> 
> Signed-off-by: Eric Paris <[email protected]>

With the fix already noted by James,

Acked-by:  Stephen Smalley <[email protected]>

Note that you need to also submit a patch for policy to reserve that
class to avoid future collisions.

> 
> ---
> 
>  Documentation/sysctl/kernel.txt                  |   14 ++++++++++++++
>  include/linux/security.h                         |   17 ++++++++++++-----
>  kernel/sysctl.c                                  |    8 ++++++++
>  mm/mmap.c                                        |    4 ++--
>  mm/mremap.c                                      |   13 +++++++++++--
>  mm/nommu.c                                       |    2 +-
>  security/dummy.c                                 |    6 +++++-
>  security/security.c                              |    2 ++
>  security/selinux/hooks.c                         |   12 ++++++++----
>  security/selinux/include/av_perm_to_string.h     |    1 +
>  security/selinux/include/av_permissions.h        |    1 +
>  security/selinux/include/class_to_string.h       |    1 +
>  security/selinux/include/flask.h                 |    1 +
>  13 files changed, 67 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 111fd28..be3991c 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -29,6 +29,7 @@ show up in /proc/sys/kernel:
>  - java-interpreter            [ binfmt_java, obsolete ]
>  - kstack_depth_to_print       [ X86 only ]
>  - l2cr                        [ PPC only ]
> +- mmap_min_addr
>  - modprobe                    ==> Documentation/kmod.txt
>  - msgmax
>  - msgmnb
> @@ -178,6 +179,19 @@ kernel stack.
>  
>  ==============================================================
>  
> +mmap_min_addr
> +
> +This file indicates the amount of address space  which a user process will be
> +restricted from mmaping.  Since kernel null dereference bugs could
> +accidentally operate based on the information in the first couple of pages of
> +memory userspace processes should not be allowed to write to them.  By default
> +this value is set to 0 and no protections will be enforced by the security
> +module.  Setting this value to something like 64k will allow the vast majority
> +of applications to work correctly and provide defense in depth against future
> +potential kernel bugs.
> +
> +==============================================================
> +
>  osrelease, ostype & version:
>  
>  # cat osrelease
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 9eb9e0f..c11dc8a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -71,6 +71,7 @@ struct xfrm_user_sec_ctx;
>  extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb);
>  extern int cap_netlink_recv(struct sk_buff *skb, int cap);
>  
> +extern unsigned long mmap_min_addr;
>  /*
>   * Values used in the task_security_ops calls
>   */
> @@ -1241,8 +1242,9 @@ struct security_operations {
>  	int (*file_ioctl) (struct file * file, unsigned int cmd,
>  			   unsigned long arg);
>  	int (*file_mmap) (struct file * file,
> -			  unsigned long reqprot,
> -			  unsigned long prot, unsigned long flags);
> +			  unsigned long reqprot, unsigned long prot,
> +			  unsigned long flags, unsigned long addr,
> +			  unsigned long addr_only);
>  	int (*file_mprotect) (struct vm_area_struct * vma,
>  			      unsigned long reqprot,
>  			      unsigned long prot);
> @@ -1814,9 +1816,12 @@ static inline int security_file_ioctl (struct file *file, unsigned int cmd,
>  
>  static inline int security_file_mmap (struct file *file, unsigned long reqprot,
>  				      unsigned long prot,
> -				      unsigned long flags)
> +				      unsigned long flags,
> +				      unsigned long addr,
> +				      unsigned long addr_only)
>  {
> -	return security_ops->file_mmap (file, reqprot, prot, flags);
> +	return security_ops->file_mmap (file, reqprot, prot, flags, addr,
> +					addr_only);
>  }
>  
>  static inline int security_file_mprotect (struct vm_area_struct *vma,
> @@ -2489,7 +2494,9 @@ static inline int security_file_ioctl (struct file *file, unsigned int cmd,
>  
>  static inline int security_file_mmap (struct file *file, unsigned long reqprot,
>  				      unsigned long prot,
> -				      unsigned long flags)
> +				      unsigned long flags,
> +				      unsigned long addr,
> +				      unsigned long addr_only)
>  {
>  	return 0;
>  }
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 30ee462..a6feef2 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -615,6 +615,14 @@ static ctl_table kern_table[] = {
>  		.proc_handler   = &proc_dointvec,
>  	},
>  #endif
> +	{
> +		.ctl_name	= CTL_UNNUMBERED,
> +		.procname	= "mmap_min_addr",
> +		.data		= &mmap_min_addr,
> +		.maxlen         = sizeof(unsigned long),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec,
> +	},
>  
>  	{ .ctl_name = 0 }
>  };
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 68b9ad2..bce4995 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1023,10 +1023,10 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned long addr,
>  		}
>  	}
>  
> -	error = security_file_mmap(file, reqprot, prot, flags);
> +	error = security_file_mmap(file, reqprot, prot, flags, addr, 0);
>  	if (error)
>  		return error;
> -		
> +
>  	/* Clear old maps */
>  	error = -ENOMEM;
>  munmap_back:
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 5d4bd4f..bc7c52e 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -291,6 +291,10 @@ unsigned long do_mremap(unsigned long addr,
>  		if ((addr <= new_addr) && (addr+old_len) > new_addr)
>  			goto out;
>  
> +		ret = security_file_mmap(0, 0, 0, 0, new_addr, 1);
> +		if (ret)
> +			goto out;
> +
>  		ret = do_munmap(mm, new_addr, new_len);
>  		if (ret)
>  			goto out;
> @@ -390,8 +394,13 @@ unsigned long do_mremap(unsigned long addr,
>  
>  			new_addr = get_unmapped_area(vma->vm_file, 0, new_len,
>  						vma->vm_pgoff, map_flags);
> -			ret = new_addr;
> -			if (new_addr & ~PAGE_MASK)
> +			if (new_addr & ~PAGE_MASK) {
> +				ret = new_addr;
> +				goto out;
> +			}
> +
> +			ret = security_file_mmap(0, 0, 0, 0, new_addr, 1);
> +			if (ret)
>  				goto out;
>  		}
>  		ret = move_vma(vma, addr, old_len, new_len, new_addr);
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 2b16b00..6f8ddee 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -639,7 +639,7 @@ static int validate_mmap_request(struct file *file,
>  	}
>  
>  	/* allow the security API to have its say */
> -	ret = security_file_mmap(file, reqprot, prot, flags);
> +	ret = security_file_mmap(file, reqprot, prot, flags, addr, 0);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/security/dummy.c b/security/dummy.c
> index 8ffd764..d6a112c 100644
> --- a/security/dummy.c
> +++ b/security/dummy.c
> @@ -420,8 +420,12 @@ static int dummy_file_ioctl (struct file *file, unsigned int command,
>  
>  static int dummy_file_mmap (struct file *file, unsigned long reqprot,
>  			    unsigned long prot,
> -			    unsigned long flags)
> +			    unsigned long flags,
> +			    unsigned long addr,
> +			    unsigned long addr_only)
>  {
> +	if (addr < mmap_min_addr)
> +		return -EACCES;
>  	return 0;
>  }
>  
> diff --git a/security/security.c b/security/security.c
> index fc8601b..024484f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -24,6 +24,7 @@ extern struct security_operations dummy_security_ops;
>  extern void security_fixup_ops(struct security_operations *ops);
>  
>  struct security_operations *security_ops;	/* Initialized to NULL */
> +unsigned long mmap_min_addr;		/* 0 means no protection */
>  
>  static inline int verify(struct security_operations *ops)
>  {
> @@ -176,4 +177,5 @@ EXPORT_SYMBOL_GPL(register_security);
>  EXPORT_SYMBOL_GPL(unregister_security);
>  EXPORT_SYMBOL_GPL(mod_reg_security);
>  EXPORT_SYMBOL_GPL(mod_unreg_security);
> +EXPORT_SYMBOL_GPL(mmap_min_addr);
>  EXPORT_SYMBOL(security_ops);
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index ad8dd4e..2b44832 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2568,12 +2568,16 @@ static int file_map_prot_check(struct file *file, unsigned long prot, int shared
>  }
>  
>  static int selinux_file_mmap(struct file *file, unsigned long reqprot,
> -			     unsigned long prot, unsigned long flags)
> +			     unsigned long prot, unsigned long flags,
> +			     unsigned long addr, unsigned long addr_only)
>  {
> -	int rc;
> +	int rc = 0;
> +	u32 sid = ((struct task_security_struct*)(current->security))->sid;
>  
> -	rc = secondary_ops->file_mmap(file, reqprot, prot, flags);
> -	if (rc)
> +	if (addr < mmap_min_addr)
> +		rc = avc_has_perm(sid, sid, SECCLASS_MEMPROTECT,
> +				  MEMPROTECT__MMAP_ZERO, NULL);
> +	if (rc || addr_only)
>  		return rc;
>  
>  	if (selinux_checkreqprot)
> diff --git a/security/selinux/include/av_perm_to_string.h b/security/selinux/include/av_perm_to_string.h
> index b83e740..049bf69 100644
> --- a/security/selinux/include/av_perm_to_string.h
> +++ b/security/selinux/include/av_perm_to_string.h
> @@ -158,3 +158,4 @@
>     S_(SECCLASS_KEY, KEY__CREATE, "create")
>     S_(SECCLASS_DCCP_SOCKET, DCCP_SOCKET__NODE_BIND, "node_bind")
>     S_(SECCLASS_DCCP_SOCKET, DCCP_SOCKET__NAME_CONNECT, "name_connect")
> +   S_(SECCLASS_MEMPROTECT, MEMPROTECT__MMAP_ZERO, "mmap_zero")
> diff --git a/security/selinux/include/av_permissions.h b/security/selinux/include/av_permissions.h
> index 5fee173..eda89a2 100644
> --- a/security/selinux/include/av_permissions.h
> +++ b/security/selinux/include/av_permissions.h
> @@ -823,3 +823,4 @@
>  #define DCCP_SOCKET__NAME_BIND                    0x00200000UL
>  #define DCCP_SOCKET__NODE_BIND                    0x00400000UL
>  #define DCCP_SOCKET__NAME_CONNECT                 0x00800000UL
> +#define MEMPROTECT__MMAP_ZERO                     0x00000001UL
> diff --git a/security/selinux/include/class_to_string.h b/security/selinux/include/class_to_string.h
> index 3787990..e77de0e 100644
> --- a/security/selinux/include/class_to_string.h
> +++ b/security/selinux/include/class_to_string.h
> @@ -63,3 +63,4 @@
>      S_("key")
>      S_(NULL)
>      S_("dccp_socket")
> +    S_("memprotect")
> diff --git a/security/selinux/include/flask.h b/security/selinux/include/flask.h
> index 35f309f..a9c2b20 100644
> --- a/security/selinux/include/flask.h
> +++ b/security/selinux/include/flask.h
> @@ -49,6 +49,7 @@
>  #define SECCLASS_PACKET                                  57
>  #define SECCLASS_KEY                                     58
>  #define SECCLASS_DCCP_SOCKET                             60
> +#define SECCLASS_MEMPROTECT                              61
>  
>  /*
>   * Security identifier indices for initial entities
> 
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to [email protected] with
> the words "unsubscribe selinux" without quotes as the message.
-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux