lkml.org 
[lkml]   [2007]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] Protection for exploiting null dereference using mmap
    From
    Date
    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

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

    \
     
     \ /
      Last update: 2007-06-06 19:33    [W:0.059 / U:244.668 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site