linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Piotr Kwapulinski <kwapulinski.piotr@gmail.com>
To: akpm@linux-foundation.org
Cc: cmetcalf@ezchip.com, mszeredi@suse.cz, viro@zeniv.linux.org.uk,
	dave@stgolabs.net, kirill.shutemov@linux.intel.com,
	n-horiguchi@ah.jp.nec.com, aarcange@redhat.com, mhocko@suse.com,
	iamjoonsoo.kim@lge.com, jack@suse.cz, xiexiuqi@huawei.com,
	vbabka@suse.cz, Vineet.Gupta1@synopsys.com, oleg@redhat.com,
	riel@redhat.com, gang.chen.5i5j@gmail.com,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Piotr Kwapulinski <kwapulinski.piotr@gmail.com>
Subject: [PATCH] mm: fix incorrect behavior when process virtual address space limit is exceeded
Date: Mon, 16 Nov 2015 18:36:19 +0100	[thread overview]
Message-ID: <1447695379-14526-1-git-send-email-kwapulinski.piotr@gmail.com> (raw)

When a new virtual memory area is added to the process's virtual address
space and this vma causes the process's virtual address space limit
(RLIMIT_AS) to be exceeded then kernel behaves incorrectly. Incorrect
behavior is a result of a kernel bug. The kernel in most cases
unnecessarily scans the entire process's virtual address space trying to
find the overlapping vma with the virtual memory region being added.
The kernel incorrectly compares the MAP_FIXED flag with vm_flags variable
in mmap_region function. The vm_flags variable should not be compared
with MAP_FIXED flag. The MAP_FIXED flag has got the same numerical value
as VM_MAYREAD flag (0x10). As a result the following test
from mmap_region:

if (!(vm_flags & MAP_FIXED))
is in fact:
if (!(vm_flags & VM_MAYREAD))

The VM_MAYREAD flag is almost always set in vm_flags while MAP_FIXED
flag is not so common. The result of the above condition is somewhat
reverted.
This patch fixes this bug. It causes that the kernel tries to find the
overlapping vma only when the requested virtual memory region has got
the fixed starting virtual address (MAP_FIXED flag set).
For tile architecture Calling mmap_region with the MAP_FIXED flag only is
sufficient. However the MAP_ANONYMOUS and MAP_PRIVATE flags are passed for
the completeness of the solution.

Signed-off-by: Piotr Kwapulinski <kwapulinski.piotr@gmail.com>
---
 arch/tile/mm/elf.c | 1 +
 include/linux/mm.h | 3 ++-
 mm/mmap.c          | 7 ++++---
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/tile/mm/elf.c b/arch/tile/mm/elf.c
index 6225cc9..dae4b33 100644
--- a/arch/tile/mm/elf.c
+++ b/arch/tile/mm/elf.c
@@ -142,6 +142,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
 	if (!retval) {
 		unsigned long addr = MEM_USER_INTRPT;
 		addr = mmap_region(NULL, addr, INTRPT_SIZE,
+				   MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE,
 				   VM_READ|VM_EXEC|
 				   VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, 0);
 		if (addr > (unsigned long) -PAGE_SIZE)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 00bad77..1ae21c1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1911,7 +1911,8 @@ extern int install_special_mapping(struct mm_struct *mm,
 extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 
 extern unsigned long mmap_region(struct file *file, unsigned long addr,
-	unsigned long len, vm_flags_t vm_flags, unsigned long pgoff);
+	unsigned long len, unsigned long flags,
+	vm_flags_t vm_flags, unsigned long pgoff);
 extern unsigned long do_mmap(struct file *file, unsigned long addr,
 	unsigned long len, unsigned long prot, unsigned long flags,
 	vm_flags_t vm_flags, unsigned long pgoff, unsigned long *populate);
diff --git a/mm/mmap.c b/mm/mmap.c
index 2ce04a6..ad8b845 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1399,7 +1399,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
 			vm_flags |= VM_NORESERVE;
 	}
 
-	addr = mmap_region(file, addr, len, vm_flags, pgoff);
+	addr = mmap_region(file, addr, len, flags, vm_flags, pgoff);
 	if (!IS_ERR_VALUE(addr) &&
 	    ((vm_flags & VM_LOCKED) ||
 	     (flags & (MAP_POPULATE | MAP_NONBLOCK)) == MAP_POPULATE))
@@ -1535,7 +1535,8 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags)
 }
 
 unsigned long mmap_region(struct file *file, unsigned long addr,
-		unsigned long len, vm_flags_t vm_flags, unsigned long pgoff)
+		unsigned long len, unsigned long flags,
+		vm_flags_t vm_flags, unsigned long pgoff)
 {
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma, *prev;
@@ -1551,7 +1552,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		 * MAP_FIXED may remove pages of mappings that intersects with
 		 * requested mapping. Account for the pages it would unmap.
 		 */
-		if (!(vm_flags & MAP_FIXED))
+		if (!(flags & MAP_FIXED))
 			return -ENOMEM;
 
 		nr_pages = count_vma_pages_range(mm, addr, addr + len);
-- 
2.6.2


             reply	other threads:[~2015-11-16 17:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-16 17:36 Piotr Kwapulinski [this message]
2015-11-16 20:52 ` [PATCH] mm: fix incorrect behavior when process virtual address space limit is exceeded Michal Hocko
2015-11-17  0:33   ` Michal Hocko
2015-11-18 14:32   ` Cyril Hrubis
2015-11-17 16:19 ` Oleg Nesterov
2015-11-17 16:33   ` Oleg Nesterov
2015-11-17 17:26   ` [PATCH] mm/mmap.c: remove incorrect MAP_FIXED flag comparison from mmap_region Piotr Kwapulinski
2015-11-18  0:52     ` Andrew Morton
2015-11-18 16:29       ` Piotr Kwapulinski
2015-11-20 16:38         ` [PATCH v2 1/2] mm: fix incorrect behavior when process virtual address space limit is exceeded Piotr Kwapulinski
2015-11-20 16:42         ` [PATCH v2 2/2] mm/mmap.c: remove incorrect MAP_FIXED flag comparison from mmap_region Piotr Kwapulinski
2015-11-23  8:19           ` Michal Hocko
2015-11-23 17:36             ` [PATCH v3] " Piotr Kwapulinski
2015-11-23 22:14               ` Andrew Morton
2015-11-24 16:12                 ` Piotr Kwapulinski
2015-11-27  5:27               ` Naoya Horiguchi
2015-11-17 17:38   ` [PATCH] mm: fix incorrect behavior when process virtual address space limit is exceeded Chris Metcalf
2015-11-17 19:03     ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1447695379-14526-1-git-send-email-kwapulinski.piotr@gmail.com \
    --to=kwapulinski.piotr@gmail.com \
    --cc=Vineet.Gupta1@synopsys.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cmetcalf@ezchip.com \
    --cc=dave@stgolabs.net \
    --cc=gang.chen.5i5j@gmail.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jack@suse.cz \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=mszeredi@suse.cz \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=oleg@redhat.com \
    --cc=riel@redhat.com \
    --cc=vbabka@suse.cz \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xiexiuqi@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).