From 0955dc03aedfb6a5565445b3f2176255b784cc6a Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Wed, 21 Nov 2007 09:01:36 -0500 Subject: SELinux: do not clear f_op when removing entries Do not clear f_op when removing entries since it isn't safe to do. Signed-off-by: Stephen Smalley Signed-off-by: James Morris --- security/selinux/selinuxfs.c | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index f5f3e6da5da..ac6fe99bd32 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -838,10 +838,6 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf, ret = -EFAULT; - /* check to see if this file has been deleted */ - if (!filep->f_op) - goto out; - if (count > PAGE_SIZE) { ret = -EINVAL; goto out; @@ -882,10 +878,6 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, if (length) goto out; - /* check to see if this file has been deleted */ - if (!filep->f_op) - goto out; - if (count >= PAGE_SIZE) { length = -ENOMEM; goto out; @@ -940,10 +932,6 @@ static ssize_t sel_commit_bools_write(struct file *filep, if (length) goto out; - /* check to see if this file has been deleted */ - if (!filep->f_op) - goto out; - if (count >= PAGE_SIZE) { length = -ENOMEM; goto out; @@ -982,11 +970,9 @@ static const struct file_operations sel_commit_bools_ops = { .write = sel_commit_bools_write, }; -/* partial revoke() from fs/proc/generic.c proc_kill_inodes */ static void sel_remove_entries(struct dentry *de) { - struct list_head *p, *node; - struct super_block *sb = de->d_sb; + struct list_head *node; spin_lock(&dcache_lock); node = de->d_subdirs.next; @@ -1006,18 +992,6 @@ static void sel_remove_entries(struct dentry *de) } spin_unlock(&dcache_lock); - - file_list_lock(); - list_for_each(p, &sb->s_files) { - struct file * filp = list_entry(p, struct file, f_u.fu_list); - struct dentry * dentry = filp->f_path.dentry; - - if (dentry->d_parent != de) { - continue; - } - filp->f_op = NULL; - } - file_list_unlock(); } #define BOOL_DIR_NAME "booleans" -- cgit v1.2.3 From d313f948309ab22797316e789a7ff8fa358176b6 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Mon, 26 Nov 2007 11:12:53 -0500 Subject: SELinux: detect dead booleans Instead of using f_op to detect dead booleans, check the inode index against the number of booleans and check the dentry name against the boolean name for that index on reads and writes. This prevents incorrect use of a boolean file opened prior to a policy reload while allowing valid use of it as long as it still corresponds to the same boolean in the policy. Signed-off-by: Stephen Smalley Signed-off-by: James Morris --- security/selinux/selinuxfs.c | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index ac6fe99bd32..2fa483f2611 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -65,6 +65,7 @@ static DEFINE_MUTEX(sel_mutex); /* global data for booleans */ static struct dentry *bool_dir = NULL; static int bool_num = 0; +static char **bool_pending_names; static int *bool_pending_values = NULL; /* global data for classes */ @@ -832,11 +833,16 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf, ssize_t length; ssize_t ret; int cur_enforcing; - struct inode *inode; + struct inode *inode = filep->f_path.dentry->d_inode; + unsigned index = inode->i_ino & SEL_INO_MASK; + const char *name = filep->f_path.dentry->d_name.name; mutex_lock(&sel_mutex); - ret = -EFAULT; + if (index >= bool_num || strcmp(name, bool_pending_names[index])) { + ret = -EINVAL; + goto out; + } if (count > PAGE_SIZE) { ret = -EINVAL; @@ -847,15 +853,13 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf, goto out; } - inode = filep->f_path.dentry->d_inode; - cur_enforcing = security_get_bool_value(inode->i_ino&SEL_INO_MASK); + cur_enforcing = security_get_bool_value(index); if (cur_enforcing < 0) { ret = cur_enforcing; goto out; } - length = scnprintf(page, PAGE_SIZE, "%d %d", cur_enforcing, - bool_pending_values[inode->i_ino&SEL_INO_MASK]); + bool_pending_values[index]); ret = simple_read_from_buffer(buf, count, ppos, page, length); out: mutex_unlock(&sel_mutex); @@ -868,9 +872,11 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, size_t count, loff_t *ppos) { char *page = NULL; - ssize_t length = -EFAULT; + ssize_t length; int new_value; - struct inode *inode; + struct inode *inode = filep->f_path.dentry->d_inode; + unsigned index = inode->i_ino & SEL_INO_MASK; + const char *name = filep->f_path.dentry->d_name.name; mutex_lock(&sel_mutex); @@ -878,12 +884,19 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, if (length) goto out; + if (index >= bool_num || strcmp(name, bool_pending_names[index])) { + length = -EINVAL; + goto out; + } + if (count >= PAGE_SIZE) { length = -ENOMEM; goto out; } + if (*ppos != 0) { /* No partial writes. */ + length = -EINVAL; goto out; } page = (char*)get_zeroed_page(GFP_KERNEL); @@ -892,6 +905,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, goto out; } + length = -EFAULT; if (copy_from_user(page, buf, count)) goto out; @@ -902,8 +916,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf, if (new_value) new_value = 1; - inode = filep->f_path.dentry->d_inode; - bool_pending_values[inode->i_ino&SEL_INO_MASK] = new_value; + bool_pending_values[index] = new_value; length = count; out: @@ -923,7 +936,7 @@ static ssize_t sel_commit_bools_write(struct file *filep, size_t count, loff_t *ppos) { char *page = NULL; - ssize_t length = -EFAULT; + ssize_t length; int new_value; mutex_lock(&sel_mutex); @@ -946,6 +959,7 @@ static ssize_t sel_commit_bools_write(struct file *filep, goto out; } + length = -EFAULT; if (copy_from_user(page, buf, count)) goto out; @@ -1010,7 +1024,9 @@ static int sel_make_bools(void) u32 sid; /* remove any existing files */ + kfree(bool_pending_names); kfree(bool_pending_values); + bool_pending_names = NULL; bool_pending_values = NULL; sel_remove_entries(dir); @@ -1052,16 +1068,17 @@ static int sel_make_bools(void) d_add(dentry, inode); } bool_num = num; + bool_pending_names = names; bool_pending_values = values; out: free_page((unsigned long)page); + return ret; +err: if (names) { for (i = 0; i < num; i++) kfree(names[i]); kfree(names); } - return ret; -err: kfree(values); sel_remove_entries(dir); ret = -ENOMEM; -- cgit v1.2.3 From ab5a91a8364c3d6fc617abc47cc81d162c01d90a Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 26 Nov 2007 18:47:46 -0500 Subject: Security: allow capable check to permit mmap or low vm space On a kernel with CONFIG_SECURITY but without an LSM which implements security_file_mmap it is impossible for an application to mmap addresses lower than mmap_min_addr. Based on a suggestion from a developer in the openwall community this patch adds a check for CAP_SYS_RAWIO. It is assumed that any process with this capability can harm the system a lot more easily than writing some stuff on the zero page and then trying to get the kernel to trip over itself. It also means that programs like X on i686 which use vm86 emulation can work even with mmap_min_addr set. Signed-off-by: Eric Paris Signed-off-by: James Morris --- security/dummy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/dummy.c b/security/dummy.c index 6d895ade73d..3ccfbbe973b 100644 --- a/security/dummy.c +++ b/security/dummy.c @@ -426,7 +426,7 @@ static int dummy_file_mmap (struct file *file, unsigned long reqprot, unsigned long addr, unsigned long addr_only) { - if (addr < mmap_min_addr) + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) return -EACCES; return 0; } -- cgit v1.2.3 From 8869477a49c3e99def1fcdadd6bbc407fea14b45 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 26 Nov 2007 18:47:26 -0500 Subject: security: protect from stack expantion into low vm addresses Add security checks to make sure we are not attempting to expand the stack into memory protected by mmap_min_addr Signed-off-by: Eric Paris Signed-off-by: James Morris --- mm/mmap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index facc1a75bd4..938313c76d0 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1615,6 +1615,12 @@ static inline int expand_downwards(struct vm_area_struct *vma, */ if (unlikely(anon_vma_prepare(vma))) return -ENOMEM; + + address &= PAGE_MASK; + error = security_file_mmap(0, 0, 0, 0, address, 1); + if (error) + return error; + anon_vma_lock(vma); /* @@ -1622,8 +1628,6 @@ static inline int expand_downwards(struct vm_area_struct *vma, * is required to hold the mmap_sem in read mode. We need the * anon_vma lock to serialize against concurrent expand_stacks. */ - address &= PAGE_MASK; - error = 0; /* Somebody else might have raced and expanded it already */ if (address < vma->vm_start) { -- cgit v1.2.3 From 7cd94146cd504016315608e297219f9fb7b1413b Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 26 Nov 2007 18:47:40 -0500 Subject: Security: round mmap hint address above mmap_min_addr If mmap_min_addr is set and a process attempts to mmap (not fixed) with a non-null hint address less than mmap_min_addr the mapping will fail the security checks. Since this is just a hint address this patch will round such a hint address above mmap_min_addr. gcj was found to try to be very frugal with vm usage and give hint addresses in the 8k-32k range. Without this patch all such programs failed and with the patch they happily get a higher address. This patch is wrappad in CONFIG_SECURITY since mmap_min_addr doesn't exist without it and there would be no security check possible no matter what. So we should not bother compiling in this rounding if it is just a waste of time. Signed-off-by: Eric Paris Signed-off-by: James Morris --- include/linux/mm.h | 16 ++++++++++++++++ mm/mmap.c | 3 +++ mm/nommu.c | 3 +++ 3 files changed, 22 insertions(+) diff --git a/include/linux/mm.h b/include/linux/mm.h index 520238cbae5..1b7b95c67ac 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -12,6 +12,7 @@ #include #include #include +#include struct mempolicy; struct anon_vma; @@ -512,6 +513,21 @@ static inline void set_page_links(struct page *page, enum zone_type zone, set_page_section(page, pfn_to_section_nr(pfn)); } +/* + * If a hint addr is less than mmap_min_addr change hint to be as + * low as possible but still greater than mmap_min_addr + */ +static inline unsigned long round_hint_to_min(unsigned long hint) +{ +#ifdef CONFIG_SECURITY + hint &= PAGE_MASK; + if (((void *)hint != NULL) && + (hint < mmap_min_addr)) + return PAGE_ALIGN(mmap_min_addr); +#endif + return hint; +} + /* * Some inline functions in vmstat.h depend on page_zone() */ diff --git a/mm/mmap.c b/mm/mmap.c index 938313c76d0..f4cfc6ac08d 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -912,6 +912,9 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned long addr, if (!len) return -EINVAL; + if (!(flags & MAP_FIXED)) + addr = round_hint_to_min(addr); + error = arch_mmap_check(addr, len, flags); if (error) return error; diff --git a/mm/nommu.c b/mm/nommu.c index 35622c59092..b989cb928a7 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -829,6 +829,9 @@ unsigned long do_mmap_pgoff(struct file *file, void *result; int ret; + if (!(flags & MAP_FIXED)) + addr = round_hint_to_min(addr); + /* decide whether we should attempt the mapping, and if so what sort of * mapping */ ret = validate_mmap_request(file, addr, len, prot, flags, pgoff, -- cgit v1.2.3 From 5a211a5deabcafdc764817d5b4510c767d317ddc Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 4 Dec 2007 11:06:55 -0500 Subject: VM/Security: add security hook to do_brk Given a specifically crafted binary do_brk() can be used to get low pages available in userspace virtually memory and can thus be used to circumvent the mmap_min_addr low memory protection. Add security checks in do_brk(). Signed-off-by: Eric Paris Acked-by: Alan Cox Signed-off-by: James Morris --- mm/mmap.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mm/mmap.c b/mm/mmap.c index f4cfc6ac08d..15678aa6ec7 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1941,6 +1941,10 @@ unsigned long do_brk(unsigned long addr, unsigned long len) if (is_hugepage_only_range(mm, addr, len)) return -EINVAL; + error = security_file_mmap(0, 0, 0, 0, addr, 1); + if (error) + return error; + flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags; error = arch_mmap_check(addr, len, flags); -- cgit v1.2.3