[PATCH] SELinux protection for exploiting null dereference using mmap

[PATCH] SELinux protection for exploiting null dereference using mmap

[Eric Paris]

Assuming there is a kernel bug which includes a null dereference it 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 creates a new
LSM/SELinux hook which checks mmap operations to see if the user is
attempting to mmap the first page.  Other options might be to simply
disallow these operations completely through something like a sysctl but
the fine grained approach of SELinux makes it possible that an
application which actually requires this permission is allowed to
function while helping to reduce the impact of null dereference kernel
bugs in the future.

I'm willing to take a different approach to the problem if anyone has
any other suggestions, but I think that making the kernel a bit more
resilient to null dereference bugs is a good idea.

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

---

 include/linux/security.h                     |   11 +++++++++++
 mm/mmap.c                                    |    6 ++++++
 mm/mremap.c                                  |    6 ++++++
 mm/nommu.c                                   |    6 ++++++
 security/dummy.c                             |    6 ++++++
 security/selinux/hooks.c                     |    9 +++++++++
 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 +
 10 files changed, 48 insertions(+)

diff --git a/include/linux/security.h b/include/linux/security.h
index 9eb9e0f..11003cd 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1240,6 +1240,7 @@ struct security_operations {
 	void (*file_free_security) (struct file * file);
 	int (*file_ioctl) (struct file * file, unsigned int cmd,
 			   unsigned long arg);
+	int (*mmap_zero) (void);
 	int (*file_mmap) (struct file * file,
 			  unsigned long reqprot,
 			  unsigned long prot, unsigned long flags);
@@ -1812,6 +1813,11 @@ static inline int security_file_ioctl (struct file *file, unsigned int cmd,
 	return security_ops->file_ioctl (file, cmd, arg);
 }
 
+static inline int security_mmap_zero (void)
+{
+	return security_ops->mmap_zero ();
+}
+
 static inline int security_file_mmap (struct file *file, unsigned long reqprot,
 				      unsigned long prot,
 				      unsigned long flags)
@@ -2487,6 +2493,11 @@ static inline int security_file_ioctl (struct file *file, unsigned int cmd,
 	return 0;
 }
 
+static inline int security_mmap_zero (void)
+{
+	return 0;
+}
+
 static inline int security_file_mmap (struct file *file, unsigned long reqprot,
 				      unsigned long prot,
 				      unsigned long flags)
diff --git a/mm/mmap.c b/mm/mmap.c
index 68b9ad2..3c2c3f3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1026,6 +1026,12 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned long addr,
 	error = security_file_mmap(file, reqprot, prot, flags);
 	if (error)
 		return error;
+
+	if (unlikely(addr < PAGE_SIZE)) {
+		error = security_mmap_zero();
+		if (error)
+			return error;
+	}
 		
 	/* Clear old maps */
 	error = -ENOMEM;
diff --git a/mm/mremap.c b/mm/mremap.c
index 5d4bd4f..9e76313 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -291,6 +291,12 @@ unsigned long do_mremap(unsigned long addr,
 		if ((addr <= new_addr) && (addr+old_len) > new_addr)
 			goto out;
 
+		if (unlikely(new_addr < PAGE_SIZE)) {
+			ret = security_mmap_zero();
+			if (ret)
+				goto out;
+			ret = -EINVAL;
+		}
 		ret = do_munmap(mm, new_addr, new_len);
 		if (ret)
 			goto out;
diff --git a/mm/nommu.c b/mm/nommu.c
index 2b16b00..eb6df72 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -643,6 +643,12 @@ static int validate_mmap_request(struct file *file,
 	if (ret < 0)
 		return ret;
 
+	if (unlikely(addr < PAGE_SIZE)) {
+		ret = security_mmap_zero();
+		if (ret)
+			return ret;
+	}
+
 	/* looks okay */
 	*_capabilities = capabilities;
 	return 0;
diff --git a/security/dummy.c b/security/dummy.c
index 8ffd764..13e30aa 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -418,6 +418,11 @@ static int dummy_file_ioctl (struct file *file, unsigned int command,
 	return 0;
 }
 
+static int dummy_mmap_zero (void)
+{
+	return 0;
+}
+
 static int dummy_file_mmap (struct file *file, unsigned long reqprot,
 			    unsigned long prot,
 			    unsigned long flags)
@@ -1022,6 +1027,7 @@ void security_fixup_ops (struct security_operations *ops)
 	set_to_dummy_if_null(ops, file_alloc_security);
 	set_to_dummy_if_null(ops, file_free_security);
 	set_to_dummy_if_null(ops, file_ioctl);
+	set_to_dummy_if_null(ops, mmap_zero);
 	set_to_dummy_if_null(ops, file_mmap);
 	set_to_dummy_if_null(ops, file_mprotect);
 	set_to_dummy_if_null(ops, file_lock);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad8dd4e..0166d5e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2567,6 +2567,14 @@ static int file_map_prot_check(struct file *file, unsigned long prot, int shared
 	return 0;
 }
 
+static int selinux_mmap_zero(void)
+{
+	u32 sid = ((struct task_security_struct*)(current->security))->sid;
+
+	return avc_has_perm(sid, sid, SECCLASS_PROCESS_SPECIAL,
+			    PROCESS_SPECIAL__MMAP_ZERO, NULL);
+}
+
 static int selinux_file_mmap(struct file *file, unsigned long reqprot,
 			     unsigned long prot, unsigned long flags)
 {
@@ -4772,6 +4780,7 @@ static struct security_operations selinux_ops = {
 	.file_alloc_security =		selinux_file_alloc_security,
 	.file_free_security =		selinux_file_free_security,
 	.file_ioctl =			selinux_file_ioctl,
+	.mmap_zero =			selinux_mmap_zero,
 	.file_mmap =			selinux_file_mmap,
 	.file_mprotect =		selinux_file_mprotect,
 	.file_lock =			selinux_file_lock,
diff --git a/security/selinux/include/av_perm_to_string.h b/security/selinux/include/av_perm_to_string.h
index b83e740..b1322ca 100644
--- a/security/selinux/include/av_perm_to_string.h
+++ b/security/selinux/include/av_perm_to_string.h
@@ -158,3 +160,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_PROCESS_SPECIAL, PROCESS_SPECIAL__MMAP_ZERO, "mmap_zero")
diff --git a/security/selinux/include/av_permissions.h b/security/selinux/include/av_permissions.h
index 5fee173..af001f8 100644
--- a/security/selinux/include/av_permissions.h
+++ b/security/selinux/include/av_permissions.h
@@ -823,3 +825,4 @@
 #define DCCP_SOCKET__NAME_BIND                    0x00200000UL
 #define DCCP_SOCKET__NODE_BIND                    0x00400000UL
 #define DCCP_SOCKET__NAME_CONNECT                 0x00800000UL
+#define PROCESS_SPECIAL__MMAP_ZERO                0x00000001UL
diff --git a/security/selinux/include/class_to_string.h b/security/selinux/include/class_to_string.h
index 3787990..802c796 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_("process_special")
diff --git a/security/selinux/include/flask.h b/security/selinux/include/flask.h
index 35f309f..274f206 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_PROCESS_SPECIAL                         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.

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[James Morris]

On Wed, 30 May 2007, Eric Paris wrote:

> +++ b/mm/mmap.c
> @@ -1026,6 +1026,12 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned long addr,
>  	error = security_file_mmap(file, reqprot, prot, flags);
>  	if (error)
>  		return error;
> +
> +	if (unlikely(addr < PAGE_SIZE)) {
> +		error = security_mmap_zero();
> +		if (error)
> +			return error;
> +	}

I think this check should be folded into the security_file_mmap hook, 
which will need an address parameter.  The code seems to have changed a 
bit upstream vs. what you're using, but I think it'll work.

A useful side effect will be removal of the address check from the 
callsites and to allow the security module to check the address 
value itself.

In the case of do_mremap(), perhaps add another flag to indicate that 
it's an address value check only?  Seems ugly, but perhaps less so than a 
special hook for this.

> +	return avc_has_perm(sid, sid, SECCLASS_PROCESS_SPECIAL,
> +			    PROCESS_SPECIAL__MMAP_ZERO, NULL);

A new class for this seems odd. 

Is this class likely to be used for other similar checks?  I think it at 
least needs a better name.


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

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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[Eric Paris]

On Wed, 2007-05-30 at 20:07 -0400, James Morris wrote:
> On Wed, 30 May 2007, Eric Paris wrote:

> > +	return avc_has_perm(sid, sid, SECCLASS_PROCESS_SPECIAL,
> > +			    PROCESS_SPECIAL__MMAP_ZERO, NULL);
> 
> A new class for this seems odd. 
> 
> Is this class likely to be used for other similar checks?  I think it at 
> least needs a better name.

suggestions for a better name from anyone?  Originally I thought of
using the process class but one of the biggest reasons for a new class
was the common occurance of grantings for 

something_t self: process *

since this isn't a permission which is actually only affecting the
process itself its a little different.  This permission is actually
limiting the effect of a kernel bug not restricting a process from
dealing with its own resources.

He's a quick patch which I only checked by booting and testing with
MAP_FIXED, no mremap testing....

It doesn't address the question of 'is 1 page enough'   Anyone with a
good suggestion of how much is enough?  Or a reasonable idea on how to
make something like this configureable if we can't agree on a single
number?

-Eric

 include/linux/security.h                         |   17 +++++++---
 mm/mmap.c                                        |    4 +-
 mm/mremap.c                                      |    4 ++
 mm/nommu.c                                       |    2 +-
 security/dummy.c                                 |    4 ++-
 security/selinux/hooks.c                         |   15 +++++++--
 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 +
 11 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 9eb9e0f..0aba455 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1240,9 +1240,11 @@ struct security_operations {
 	void (*file_free_security) (struct file * file);
 	int (*file_ioctl) (struct file * file, unsigned int cmd,
 			   unsigned long arg);
+	int (*mmap_zero) (void);
 	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/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..ecb15e4 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;
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(NULL, 0, 0, 0, addr, 1);
 	if (ret < 0)
 		return ret;
 
diff --git a/security/dummy.c b/security/dummy.c
index 8ffd764..365c4db 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -420,7 +420,9 @@ 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)
 {
 	return 0;
 }
diff --git a/security/selinux/avc.c b/security/selinux/avc.c
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad8dd4e..86ea5a5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2568,14 +2568,23 @@ 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);
+	rc = secondary_ops->file_mmap(file, reqprot, prot, flags, addr,
+				      addr_only);
 	if (rc)
 		return rc;
 
+	if (addr < PAGE_SIZE)
+		rc = avc_has_perm(sid, sid, SECCLASS_PROCESS_SPECIAL,
+				  PROCESS_SPECIAL__MMAP_ZERO, NULL);
+	if (rc || addr_only)
+		return rc;
+
 	if (selinux_checkreqprot)
 		prot = reqprot;
 
diff --git a/security/selinux/include/av_inherit.h b/security/selinux/include/av_inherit.h
diff --git a/security/selinux/include/av_perm_to_string.h b/security/selinux/include/av_perm_to_string.h
index b83e740..5f65bf2 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_PROCESS_SPECIAL, PROCESS_SPECIAL__MMAP_ZERO, "mmap_zero")
diff --git a/security/selinux/include/av_permissions.h b/security/selinux/include/av_permissions.h
index 5fee173..0ac3f63 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 PROCESS_SPECIAL__MMAP_ZERO                0x00000001UL
diff --git a/security/selinux/include/class_to_string.h b/security/selinux/include/class_to_string.h
index 3787990..802c796 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_("process_special")
diff --git a/security/selinux/include/common_perm_to_string.h b/security/selinux/include/common_perm_to_string.h
diff --git a/security/selinux/include/flask.h b/security/selinux/include/flask.h
index 35f309f..274f206 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_PROCESS_SPECIAL                         61
 
 /*
  * Security identifier indices for initial entities
diff --git a/security/selinux/include/initial_sid_to_string.h b/security/selinux/include/initial_sid_to_string.h



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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[James Morris]

On Thu, 31 May 2007, Eric Paris wrote:

> suggestions for a better name from anyone? 

system? kernel?

> It doesn't address the question of 'is 1 page enough'   Anyone with a
> good suggestion of how much is enough?  Or a reasonable idea on how to
> make something like this configureable if we can't agree on a single
> number?

For SELinux, have a default and allow the value to be set via a selinuxfs 
node.



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

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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[Stephen Smalley]

On Thu, 2007-05-31 at 09:45 -0400, James Morris wrote:
> On Thu, 31 May 2007, Eric Paris wrote:
> 
> > suggestions for a better name from anyone? 
> 
> system? kernel?

In retrospect, we likely should have moved all of the execmem/stack/heap
checks into a separate memory class and we could have put this one there
as well.  There is a 'system' class already, so it isn't better than
using 'process' aside from having more free bits; using a new class will
ensure that no existing policy implicitly allows this check via an allow
a b:c *; rule.

> > It doesn't address the question of 'is 1 page enough'   Anyone with a
> > good suggestion of how much is enough?  Or a reasonable idea on how to
> > make something like this configureable if we can't agree on a single
> > number?
> 
> For SELinux, have a default and allow the value to be set via a selinuxfs 
> node.

Or it could be a sysctl value if you wanted it to generalize beyond
selinux.

-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[Stephen Smalley]

On Thu, 2007-05-31 at 07:55 -0700, Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Stephen Smalley wrote:
> > Or it could be a sysctl value if you wanted it to generalize beyond
> > selinux.
> 
> That's likely the right answer.  But then you need to protect changing
> the value.  How does SELinux deal with that?

The policy can label sysctls based on their name (since there we can
generate a reliable stable name from kernel data that isn't subject to
userspace manipulation or aliasing, unlike typical pathnames), and then
control access based on the TE type.

-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[James Morris]

On Thu, 31 May 2007, Stephen Smalley wrote:

> On Thu, 2007-05-31 at 09:45 -0400, James Morris wrote:
> > On Thu, 31 May 2007, Eric Paris wrote:
> > 
> > > suggestions for a better name from anyone? 
> > 
> > system? kernel?
> 
> In retrospect, we likely should have moved all of the execmem/stack/heap
> checks into a separate memory class and we could have put this one there
> as well.  There is a 'system' class already, so it isn't better than
> using 'process' aside from having more free bits; using a new class will
> ensure that no existing policy implicitly allows this check via an allow
> a b:c *; rule.

Time for a new policy format version?

> > > It doesn't address the question of 'is 1 page enough'   Anyone with a
> > > good suggestion of how much is enough?  Or a reasonable idea on how to
> > > make something like this configureable if we can't agree on a single
> > > number?
> > 
> > For SELinux, have a default and allow the value to be set via a selinuxfs 
> > node.
> 
> Or it could be a sysctl value if you wanted it to generalize beyond
> selinux.

So, the dummy module would use the sysctl value, and SELinux would 
selectively allow bypass of this?  We then need to ensure SELinux is 
appropriately mediating changes to the sysctl.


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

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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[Stephen Smalley]

On Thu, 2007-05-31 at 11:19 -0400, James Morris wrote:
> On Thu, 31 May 2007, Stephen Smalley wrote:
> 
> > On Thu, 2007-05-31 at 09:45 -0400, James Morris wrote:
> > > On Thu, 31 May 2007, Eric Paris wrote:
> > > 
> > > > suggestions for a better name from anyone? 
> > > 
> > > system? kernel?
> > 
> > In retrospect, we likely should have moved all of the execmem/stack/heap
> > checks into a separate memory class and we could have put this one there
> > as well.  There is a 'system' class already, so it isn't better than
> > using 'process' aside from having more free bits; using a new class will
> > ensure that no existing policy implicitly allows this check via an allow
> > a b:c *; rule.
> 
> Time for a new policy format version?

If we were going to do that, I think we would want to bundle up a
collection of changes that would affect the kernel format, e.g.:
	purging of obsolete perms, initial sids, etc,
	reorganization of existing access vectors,
	revisit ioctl perms,
	add user_transition support,
	add native support for type sets and their relationships

But we don't really need it just for this check; adding a new class
isn't a problem.

> > > > It doesn't address the question of 'is 1 page enough'   Anyone with a
> > > > good suggestion of how much is enough?  Or a reasonable idea on how to
> > > > make something like this configureable if we can't agree on a single
> > > > number?
> > > 
> > > For SELinux, have a default and allow the value to be set via a selinuxfs 
> > > node.
> > 
> > Or it could be a sysctl value if you wanted it to generalize beyond
> > selinux.
> 
> So, the dummy module would use the sysctl value, and SELinux would 
> selectively allow bypass of this?  We then need to ensure SELinux is 
> appropriately mediating changes to the sysctl.

Possibly, although the dummy module would be limited to a global
system-wide restriction.

SELinux can already control access to sysctls; we just have to label it
appropriately in policy genfs_contexts and define the allow rules to the
type.

-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[Chris Wright]

* Eric Paris (eparis@redhat.com) wrote:
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 9eb9e0f..0aba455 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1240,9 +1240,11 @@ struct security_operations {
>  	void (*file_free_security) (struct file * file);
>  	int (*file_ioctl) (struct file * file, unsigned int cmd,
>  			   unsigned long arg);
> +	int (*mmap_zero) (void);

No longer used.

>  	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);

Could just do file_mremap, since you've got 6 args here just to pass addr.

>  	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/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..ecb15e4 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;

This is not sufficient assuming we support a cutoff greater than a
single page.  You can easily get get_unmapped_area to return something
as low as 0x1000.

> 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(NULL, 0, 0, 0, addr, 1);

Why?

>  	if (ret < 0)
>  		return ret;

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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[Chris Wright]

* Eric Paris (eparis@redhat.com) wrote:
> +	int (*mmap_zero) (void);
>  	int (*file_mmap) (struct file * file,
>  			  unsigned long reqprot,
>  			  unsigned long prot, unsigned long flags);

When I looked into this originally, I expected to use existing file_mmap
hook, augmented with addr (of course, adding smth for mremap as well).

> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1026,6 +1026,12 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned long addr,
>  	error = security_file_mmap(file, reqprot, prot, flags);
>  	if (error)
>  		return error;
> +
> +	if (unlikely(addr < PAGE_SIZE)) {
> +		error = security_mmap_zero();
> +		if (error)
> +			return error;
> +	}

I believe this is correct (despite the distinction of MAP_FIXED vs
!MAP_FIXED).

> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -291,6 +291,12 @@ unsigned long do_mremap(unsigned long addr,
>  		if ((addr <= new_addr) && (addr+old_len) > new_addr)
>  			goto out;
>  
> +		if (unlikely(new_addr < PAGE_SIZE)) {
> +			ret = security_mmap_zero();
> +			if (ret)
> +				goto out;
> +			ret = -EINVAL;
> +		}

This, on the other hand, I believe is correct only to the degree that
the current semantics of get_unmapped_area stay unchanged.  This is why
I had expected passing the addr to the module directly, since the cutoff
point is a lot like policy (one page is somewhat arbitrary).

thanks,
-chris

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

Re: [PATCH] SELinux protection for exploiting null dereference using mmap

[Stephen Smalley]

On Sun, 2007-06-03 at 16:56 -0400, Alan Cox wrote:
> On Wed, May 30, 2007 at 05:48:33PM -0400, Eric Paris wrote:
> > Assuming there is a kernel bug which includes a null dereference it 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 creates a new
> 
> Gak
> 
> > +	if (unlikely(addr < PAGE_SIZE)) {
> > +		error = security_mmap_zero();
> > +		if (error)
> > +			return error;
> > +	}
> 
> Wouldn't it be better simply to pass the address and let the mmap test
> for SELinux make a decsion on it. People who care about performance don't
> run SELinux enabled kernels anyway.

That's the wrong mindset IMHO - whatever performance issues exist in
SELinux should be addressed to enable people who care about performance
to run with it enabled.

-- 
Stephen Smalley
National Security Agency


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

[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



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

[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>

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

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


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

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


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

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



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

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>

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

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


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

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


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

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>

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

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.

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

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.

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

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


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

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


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

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


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

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