| Message ID | 20090721230339.20180.99803.stgit@paris.rdu.redhat.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series |
|
| Related | show |
On Tue, 21 Jul 2009, Eric Paris wrote: > + > + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) > + return -EACCES; Please make this check a static inline. Less chance of someone breaking it, or it having some subtle bug, and easier to maintain. > address &= PAGE_MASK; > + Whitespace leak. - James
On Tue, 21 Jul 2009, Eric Paris wrote: > error = security_file_mmap(file, reqprot, prot, flags, addr, 0); > if (error) > return error; > + > + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) > + return -EACCES; > + These DAC checks should happen before the LSM hook, in keeping with the general design goal of LSM of "DAC before MAC", so that application behavior remains as consistent as possible. - James
On Wed, 22 Jul 2009, James Morris wrote: > On Tue, 21 Jul 2009, Eric Paris wrote: > > > error = security_file_mmap(file, reqprot, prot, flags, addr, 0); > > if (error) > > return error; > > + > > + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) > > + return -EACCES; > > + > > These DAC checks should happen before the LSM hook, in keeping with the > general design goal of LSM of "DAC before MAC", so that application > behavior remains as consistent as possible. Could they be moved out of core code? mmap_min_addr is already a strange feature. Now we adding something on top of it. -- 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/
diff --git a/include/linux/security.h b/include/linux/security.h index 1459091..f7d198a 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -2197,8 +2197,6 @@ static inline int security_file_mmap(struct file *file, unsigned long reqprot, unsigned long addr, unsigned long addr_only) { - if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) - return -EACCES; return 0; } diff --git a/mm/mmap.c b/mm/mmap.c index 34579b2..3bc88c4 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1050,6 +1050,10 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr, error = security_file_mmap(file, reqprot, prot, flags, addr, 0); if (error) return error; + + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) + return -EACCES; + error = ima_file_mmap(file, prot); if (error) return error; @@ -1657,10 +1661,14 @@ static int expand_downwards(struct vm_area_struct *vma, return -ENOMEM; address &= PAGE_MASK; + error = security_file_mmap(NULL, 0, 0, 0, address, 1); if (error) return error; + if ((address < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) + return -EACCES; + anon_vma_lock(vma); /* @@ -2002,6 +2010,9 @@ unsigned long do_brk(unsigned long addr, unsigned long len) if (error) return error; + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) + return -EACCES; + flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags; error = arch_mmap_check(addr, len, flags); diff --git a/mm/mremap.c b/mm/mremap.c index a39b7b9..fc866c3 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -303,6 +303,10 @@ unsigned long do_mremap(unsigned long addr, if (ret) goto out; + ret = -EACCES; + if ((new_addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) + goto out; + ret = do_munmap(mm, new_addr, new_len); if (ret) goto out; @@ -410,6 +414,10 @@ unsigned long do_mremap(unsigned long addr, ret = security_file_mmap(NULL, 0, 0, 0, new_addr, 1); if (ret) goto out; + + ret = -EACCES; + if ((new_addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) + goto out; } ret = move_vma(vma, addr, old_len, new_len, new_addr); } diff --git a/mm/nommu.c b/mm/nommu.c index 53cab10..891ed70 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -999,6 +999,9 @@ static int validate_mmap_request(struct file *file, if (ret < 0) return ret; + if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) + return -EACCES; + /* looks okay */ *_capabilities = capabilities; return 0; diff --git a/security/capability.c b/security/capability.c index f218dd3..a3a5d9b 100644 --- a/security/capability.c +++ b/security/capability.c @@ -334,8 +334,6 @@ static int cap_file_mmap(struct file *file, unsigned long reqprot, unsigned long prot, unsigned long flags, unsigned long addr, unsigned long addr_only) { - if ((addr < mmap_min_addr) && !capable(CAP_SYS_RAWIO)) - return -EACCES; return 0; }
Currently non-SELinux systems need CAP_SYS_RAWIO for an application to mmap the 0 page. On SELinux systems they need a specific SELinux permission, but do not need CAP_SYS_RAWIO. This has proved to be a poor decision by the SELinux team as, by default, SELinux users are logged in unconfined and thus a malicious non-root has nothing stopping them from mapping the 0 page of virtual memory. On a non-SELinux system, a malicious non-root user is unable to do this, as they need CAP_SYS_RAWIO. This patch checks CAP_SYS_RAWIO for all operations which attemt to map a page below mmap_min_addr. Signed-off-by: Eric Paris <eparis@redhat.com> --- include/linux/security.h | 2 -- mm/mmap.c | 11 +++++++++++ mm/mremap.c | 8 ++++++++ mm/nommu.c | 3 +++ security/capability.c | 2 -- 5 files changed, 22 insertions(+), 4 deletions(-) -- 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/