[PATCH] Protection for exploiting null dereference using mmap

[PATCH] Protection for exploiting null dereference using mmap

[Eric Paris]

Assuming there is a kernel bug which includes a null dereference that
bug may allow for a process to place information on the first page on
the system and get the kernel to act in unintended ways.  This patch
adds a new security check on mmap operations to see if the user is
attempting to mmap to low area of the address space.  The amount of
space protected is indicated by the new proc
tunable /proc/sys/kernel/mmap_protect_memory and defaults to 64k.
Setting this value to 0 will disable the checks allowing a system to
function exactly the way it does today.  This patch simply makes the
kernel more resilient in the face of future unknown null dereference
bugs.

The security checks is enforced in 2 places.  First in SELinux and also
in the dummy security function.  Doing the check in SELinux allows an
SELinux system to use its fine grained security properties to
selectively allow processes to still mmap the low pages page while
denying most processes that potentially sensitive operation.
Enforcement is also done in the dummy operation dummy_file_mmap() and
will be used for enforcement on non-selinux systems.  Although, on such
a system the proc tunable must be set to 0 if any applications actually
need to mmap to the low pages.  No fine grained security means it has to
be this all or nothing approach on non-selinux systems.

One result of using the dummy hook for non-selinux kernels means that I
can't leave the generic module stacking code in the SELinux check.  If
the secondary ops are called they will always deny the operation just
like in non-selinux systems even if SELinux policy would have allowed
the action.  This patch may be the first step to removing the arbitrary
LSM module stacking code from SELinux.  I think history has shown the
arbitrary module stacking is not a good idea and eventually I want to
pull out all the secondary calls which aren't used by the capability
module, so I view this as just the first step along those lines.

This patch uses a new SELinux security class "memprotect."  Policy
already contains a number of allow rules like  a_t self:process *
(unconfined_t being one of them) which mean that putting this check in
the process class (its best current fit) would make it useless as all
user processes, which we also want to protect against, would be allowed.
By taking the memprotect name of the new class it will also make it
possible for us to move some of the other memory protect permissions out
of 'process' and into the new class next time we bump the policy version
number (which I also think is a good future idea)

-Eric

Signed-off-by: Eric Paris <eparis@redhat.com>

---

 Documentation/sysctl/kernel.txt              |   12 ++++++++++++
 include/linux/security.h                     |   17 ++++++++++++-----
 kernel/sysctl.c                              |    9 +++++++++
 mm/mmap.c                                    |    4 ++--
 mm/mremap.c                                  |   15 +++++++++++++--
 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, 68 insertions(+), 15 deletions(-)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 111fd28..ed22eee 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_protect_memory
 - modprobe                    ==> Documentation/kmod.txt
 - msgmax
 - msgmnb
@@ -178,6 +179,17 @@ kernel stack.
 
 ==============================================================
 
+mmap_protect_memory
+
+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
+the security hooks will protect the first 64k of memory. To completely disable
+this protection the value should be set to 0.
+
+==============================================================
+
 osrelease, ostype & version:
 
 # cat osrelease
diff --git a/include/linux/security.h b/include/linux/security.h
index 9eb9e0f..56b9a1b 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 int mmap_protect_memory;
 /*
  * 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..ae2d665 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -615,6 +615,15 @@ static ctl_table kern_table[] = {
 		.proc_handler   = &proc_dointvec,
 	},
 #endif
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "mmap_protect_memory",
+		.data		= &mmap_protect_memory,
+		.maxlen         = sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+		.strategy	= &sysctl_intvec,
+	},
 
 	{ .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..7a4f2cc 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,9 +394,16 @@ 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 = new_addr;
 		}
 		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..c45ed09 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_protect_memory)
+		return -EACCES;
 	return 0;
 }
 
diff --git a/security/security.c b/security/security.c
index fc8601b..492686b 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 */
+int mmap_protect_memory = 65536;	/* Cannot mmap first 64k */
 
 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_protect_memory);
 EXPORT_SYMBOL(security_ops);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad8dd4e..d9c06b0 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_protect_memory)
+		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

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

[James Morris]

On Tue, 5 Jun 2007, Eric Paris wrote:

> +extern int mmap_protect_memory;

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 
ones.


- James
-- 
James Morris
<jmorris@namei.org>

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

[Alan Cox]

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

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

[Eric Paris]

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

While I understand, there are a few users who will have problems with
this default are we really better to not provide this defense in depth
for the majority of users and let those with problems turn it off rather
than provide no defense by default?  I could even provide a different
default for SELinux and non-SELinux if anyone saw value in that?  But if
others think that off default is  best I'll send another patch shortly
with the unsigned long fix and the default set to 0.  My hope is then
that distros will figure out to turn this on.

-Eric

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

[H. Peter Anvin]

Eric Paris wrote:
> 
> While I understand, there are a few users who will have problems with
> this default are we really better to not provide this defense in depth
> for the majority of users and let those with problems turn it off rather
> than provide no defense by default?  I could even provide a different
> default for SELinux and non-SELinux if anyone saw value in that?  But if
> others think that off default is  best I'll send another patch shortly
> with the unsigned long fix and the default set to 0.  My hope is then
> that distros will figure out to turn this on.
> 

I hope not.  This breaks any hardware virtualizer.

So yes, we're better off not having this on, and require it to be
explicitly enabled by the end user.

Sorry.

	-hpa

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

[Pavel Machek]

Hi!

> > While I understand, there are a few users who will have problems with
> > this default are we really better to not provide this defense in depth
> > for the majority of users and let those with problems turn it off rather
> > than provide no defense by default?  I could even provide a different
> > default for SELinux and non-SELinux if anyone saw value in that?  But if
> > others think that off default is  best I'll send another patch shortly
> > with the unsigned long fix and the default set to 0.  My hope is then
> > that distros will figure out to turn this on.
> > 
> 
> I hope not.  This breaks any hardware virtualizer.
> 
> So yes, we're better off not having this on, and require it to be
> explicitly enabled by the end user.

You could do something like 4G+4G patches.... but performance penalty
would be ugly.

But no, we cant break userspace 'by default'.
							Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Stephen Smalley]

On Tue, 2007-06-05 at 17:28 -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
> 
> While I understand, there are a few users who will have problems with
> this default are we really better to not provide this defense in depth
> for the majority of users and let those with problems turn it off rather
> than provide no defense by default?  I could even provide a different
> default for SELinux and non-SELinux if anyone saw value in that?  But if
> others think that off default is  best I'll send another patch shortly
> with the unsigned long fix and the default set to 0.  My hope is then
> that distros will figure out to turn this on.

I'd be ok with having a different default for SELinux vs. non-SELinux,
i.e. no restrictions by default under dummy/capability, but restrict it
by default to 64k if selinux is enabled.  Then we can use policy to
grant it as needed to the specific programs.

-- 
Stephen Smalley
National Security Agency

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

[Jan Engelhardt]

On Jun 6 2007 08:47, Stephen Smalley wrote:
>
>I'd be ok with having a different default for SELinux vs. non-SELinux,
>i.e. no restrictions by default under dummy/capability, but restrict it
>by default to 64k if selinux is enabled.  Then we can use policy to
>grant it as needed to the specific programs.

640k?



	Jan
-- 

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

[Eric Paris]

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 <eparis@redhat.com>

---

 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

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

[James Morris]

On Wed, 6 Jun 2007, Eric Paris wrote:

> +	{
> +		.ctl_name	= CTL_UNNUMBERED,
> +		.procname	= "mmap_min_addr",
> +		.data		= &mmap_min_addr,
> +		.maxlen         = sizeof(unsigned long),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec,

proc_doulongvec_minmax

(I can fix this in my tree rather than a resend just for this, if
there are some acks & no other problems).


-- 
James Morris
<jmorris@namei.org>

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

[Stephen Smalley]

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 <eparis@redhat.com>

With the fix already noted by James,

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

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 majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
-- 
Stephen Smalley
National Security Agency

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

[James Morris]

On Wed, 6 Jun 2007, Stephen Smalley wrote:

> With the fix already noted by James,
> 
> Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

Final patch applied to:

git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm


Also queued there is the following patch which enables the check in 
SELinux:


Subject: [PATCH] SELinux: enable minimum address checking for mmap

Enable enable minimum address checking for mmap if not already enabled, and
disable it on exit if we enabled it. Processes will then require the
new mmap_zero permission to override the check.  Set the default value to
64KB as suggested.  If already set, the existing value will be used.

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Eric Paris <eparis@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
---
 security/selinux/hooks.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2b44832..9a8db0b 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -112,6 +112,9 @@ int selinux_enabled = 1;
 /* Original (dummy) security module. */
 static struct security_operations *original_ops = NULL;
 
+/* Did we enable minimum mmap address checking? */
+static int enabled_mmap_min_addr;
+
 /* Minimal support for a secondary security module,
    just to allow the use of the dummy or capability modules.
    The owlsm module can alternatively be used as a secondary
@@ -4912,6 +4915,16 @@ static __init int selinux_init(void)
 	sel_inode_cache = kmem_cache_create("selinux_inode_security",
 					    sizeof(struct inode_security_struct),
 					    0, SLAB_PANIC, NULL, NULL);
+
+	/*
+	 * Tasks cannot mmap below this without the mmap_zero permission.
+	 * If not enabled already, do so by setting it to 64KB.
+	 */
+	if (mmap_min_addr == 0) {
+		enabled_mmap_min_addr = 1;
+		mmap_min_addr = 65536;
+	}
+
 	avc_init();
 
 	original_ops = secondary_ops = security_ops;
@@ -5061,6 +5074,10 @@ int selinux_disable(void)
 
 	selinux_disabled = 1;
 	selinux_enabled = 0;
+	
+	/* Disable minimum mmap address check only if we enabled it */
+	if (enabled_mmap_min_addr)
+		mmap_min_addr = 0;
 
 	/* Reset security_ops to the secondary module, dummy or capability. */
 	security_ops = secondary_ops;
-- 
1.5.0.6

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

[Chris Wright]

* Eric Paris (eparis@redhat.com) 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 <eparis@redhat.com>

With James' fix and real changelog ;-)

Acked-by: Chris Wright <chrisw@sous-sol.org>

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

[Adam Jackson]

Eric Paris <eparis <at> redhat.com> writes:
> 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!

X also needs to be able to map at zero for vm86, which makes this a lot less
useful.  In particular, the interrupt vectors need to point to the right place
within the VBIOS we're calling into, so we have to be able to map the zero page.
(And often write to it, if we're posting a non-primary card.)  Obviously this
isn't so much an issue for non-x86, where you have to use the emulator anyway.
And I'm pretty convinced that calling vm86 in a SMP environment is just suicide
so you _ought_ to use the emulator even on real x86.

We've already got the infrastructure in place to completely fake the real mode
address space for the emulator.  IWBNI we had some way of presenting a set of
virtual-address-space pages as contiguous to the vm86 task, since if we had that
we could just malloc 1M, copy in the bits we need, and go.  I don't know if vm86
mode supports that in silicon; it certainly isn't exposed in the API.  Anyone
know?

Actually it's a little worse than that.  Some chips have an escape hatch where
they remap banks of registers into the 64k VGA aperture, and the BIOS relies on
this.  So some of the pages need to be backed by device mappings, and some by
plain memory.  You'd really need either an array of pointers to userspace pages,
or a trap handler like how vm86 handles I/O cycles.  Again, I don't know offhand
whether vm86 can do this, but if it does then fixing X to take advantage of it
is pretty easy.

- ajax

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

[Chris Wright]

* Eric Paris (eparis@redhat.com) wrote:
> +mmap_protect_memory

I'm terrible at names, but something like mmap_minimum_addr would be a
little clearer at describing that it's a lower bound on mapping memory.
BTW, this is also an arch specific issue, those with disjoint kernel
and user memory space don't suffer (yet another reason to default to 0).

> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -615,6 +615,15 @@ static ctl_table kern_table[] = {
>  		.proc_handler   = &proc_dointvec,
>  	},
>  #endif
> +	{
> +		.ctl_name	= CTL_UNNUMBERED,
> +		.procname	= "mmap_protect_memory",
> +		.data		= &mmap_protect_memory,
> +		.maxlen         = sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec,
> +		.strategy	= &sysctl_intvec,

I don't think this strategy does anything without some boundary values.

> --- 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,9 +394,16 @@ 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 = new_addr;

Nit: unnecessary assignment...

>  		}
>  		ret = move_vma(vma, addr, old_len, new_len, new_addr);
		^^^
...as it's overwritten immediately after.

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

[Chris Wright]

* Eric Paris (eparis@redhat.com) wrote:
> One result of using the dummy hook for non-selinux kernels means that I
> can't leave the generic module stacking code in the SELinux check.  If
> the secondary ops are called they will always deny the operation just
> like in non-selinux systems even if SELinux policy would have allowed
> the action.  This patch may be the first step to removing the arbitrary
> LSM module stacking code from SELinux.  I think history has shown the
> arbitrary module stacking is not a good idea and eventually I want to
> pull out all the secondary calls which aren't used by the capability
> module, so I view this as just the first step along those lines.

Or replace them all with direct library calls to the capability code.

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

[Stephen Smalley]

On Tue, 2007-06-05 at 15:53 -0700, Chris Wright wrote:
> * Eric Paris (eparis@redhat.com) wrote:
> > One result of using the dummy hook for non-selinux kernels means that I
> > can't leave the generic module stacking code in the SELinux check.  If
> > the secondary ops are called they will always deny the operation just
> > like in non-selinux systems even if SELinux policy would have allowed
> > the action.  This patch may be the first step to removing the arbitrary
> > LSM module stacking code from SELinux.  I think history has shown the
> > arbitrary module stacking is not a good idea and eventually I want to
> > pull out all the secondary calls which aren't used by the capability
> > module, so I view this as just the first step along those lines.
> 
> Or replace them all with direct library calls to the capability code.

The only tricky part there is retaining the support for falling back on
capabilities upon runtime disable of selinux by /sbin/init.

-- 
Stephen Smalley
National Security Agency

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

[Russell Coker]

On Wednesday 06 June 2007 06:34, Eric Paris <eparis@redhat.com> wrote:
> This patch uses a new SELinux security class "memprotect."  Policy
> already contains a number of allow rules like  a_t self:process *
> (unconfined_t being one of them) which mean that putting this check in
> the process class (its best current fit) would make it useless as all

I think it would be best to use the process class and change the "*" rules to 
~{ memprotect }.

-- 
russell@coker.com.au
http://etbe.coker.com.au/          My Blog

http://www.coker.com.au/sponsorship.html Sponsoring Free Software development

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

[Stephen Smalley]

On Wed, 2007-06-06 at 19:01 +1000, Russell Coker wrote:
> On Wednesday 06 June 2007 06:34, Eric Paris <eparis@redhat.com> wrote:
> > This patch uses a new SELinux security class "memprotect."  Policy
> > already contains a number of allow rules like  a_t self:process *
> > (unconfined_t being one of them) which mean that putting this check in
> > the process class (its best current fit) would make it useless as all
> 
> I think it would be best to use the process class and change the "*" rules to 
> ~{ memprotect }.

Eric originally used process class, but I asked him to put it into a
separate class.  I think that current refpolicy actually doesn't have
any allow a_t self:process *; rules because we already had to refactor
all such rules when we introduced execmem and friends, but that doesn't
mean that there are not legacy policies with such rules, and I'd prefer
to isolate especially security-sensitive permissions in distinct classes
(and we are running out of room in process class).

-- 
Stephen Smalley
National Security Agency