Skip to contentSkip to conversation

mm: mlock: fix some locked_vm counting issues

PATCH Applied [email protected] 3 participants 9 messages Aug 30, 2016 – Sep 1, 2016

Subsystems (3)
Memory management Kselftest Memory mapping
lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover mm: mlock: fix some locked_vm counting issues

From: Simon Guo <[email protected]>

This patch set fixes some mlock() misbehavior when mlock()/mlock2() is invoked multiple times on intersect or same address regions.

And add selftest for this case.

Simon Guo (4):
mm: mlock: check against vma for actual mlock() size
mm: mlock: avoid increase mm->locked_vm on mlock() when already
mlock2(,MLOCK_ONFAULT)
selftest: split mlock2_ apis into separate mlock2.h selftests/vm: add test for mlock() when areas are intersected.

 mm/mlock.c                                        | 53 ++++++++++++++++
 tools/testing/selftests/vm/.gitignore             |  1 +
 tools/testing/selftests/vm/Makefile               |  4 ++
 tools/testing/selftests/vm/mlock-intersect-test.c | 76 +++++++++++++++++++++++
 tools/testing/selftests/vm/mlock2-tests.c         | 21 +------
 tools/testing/selftests/vm/mlock2.h               | 21 +++++++
 6 files changed, 156 insertions(+), 20 deletions(-)
 create mode 100644 tools/testing/selftests/vm/mlock-intersect-test.c
 create mode 100644 tools/testing/selftests/vm/mlock2.h
Signature · 7 lines
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#1 lore raw

PATCH 1/4 mm: mlock: check against vma for actual mlock() size

From: Simon Guo <[email protected]>

In do_mlock(), the check against locked memory limitation has a hole which will fail following cases at step 3):
1) User has a memory chunk from addressA with 50k, and user mem lock rlimit is 64k.
2) mlock(addressA, 30k)
3) mlock(addressA, 40k)

The 3rd step should have been allowed since the 40k request is intersected with the previous 30k at step 2), and the 3rd step is actually for mlock on the extra 10k memory.

This patch checks vma to caculate the actual "new" mlock size, if necessary, and ajust the logic to fix this issue.

Signed-off-by Simon Guo <[email protected]>
---
 mm/mlock.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
mm/mlock.c +49 -0
index 14645be..9283187 100644
@@ -617,6 +617,43 @@ static int apply_vma_lock_flags(unsigned long start, size_t len,
 	return error;
 }
 
+/*
+ * Go through vma areas and sum size of mlocked
+ * vma pages, as return value.
+ * Note deferred memory locking case(mlock2(,,MLOCK_ONFAULT)
+ * is also counted.
+ * Return value: previously mlocked page counts
+ */
+static int count_mm_mlocked_page_nr(struct mm_struct *mm,
+		unsigned long start, size_t len)
+{
+	struct vm_area_struct *vma;
+	int count = 0;
+
+	if (mm == NULL)
+		mm = current->mm;
+
+	vma = find_vma(mm, start);
+	if (vma == NULL)
+		vma = mm->mmap;
+
+	for (; vma ; vma = vma->vm_next) {
+		if (start + len <=  vma->vm_start)
+			break;
+		if (vma->vm_flags && VM_LOCKED) {
+			if (start > vma->vm_start)
+				count -= (start - vma->vm_start);
+			if (start + len < vma->vm_end) {
+				count += start + len - vma->vm_start;
+				break;
+			}
+			count += vma->vm_end - vma->vm_start;
+		}
+	}
+
+	return (PAGE_ALIGN(count) >> PAGE_SHIFT);
+}
+
 static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
 {
 	unsigned long locked;

@@ -639,6 +676,18 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
 		return -EINTR;
 
 	locked += current->mm->locked_vm;
+	if ((locked > lock_limit) && (!capable(CAP_IPC_LOCK))) {
+		/*
+		 * It is possible that the regions requested
+		 * intersect with previously mlocked areas,
+		 * that part area in "mm->locked_vm" should
+		 * not be counted to new mlock increment
+		 * count. So check and adjust locked count
+		 * if necessary.
+		 */
+		locked -= count_mm_mlocked_page_nr(current->mm,
+				start, len);
+	}
 
 	/* check against resource limits */
 	if ((locked <= lock_limit) || capable(CAP_IPC_LOCK))
Signature · 7 lines
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw

Re: [PATCH 1/4] mm: mlock: check against vma for actual mlock() size

Quoted 9 lines · On Tue, Aug 30, 2016 at 06:59:38PM +0800, [email protected] wrote:

From: Simon Guo <[email protected]>

In do_mlock(), the check against locked memory limitation has a hole which will fail following cases at step 3):
1) User has a memory chunk from addressA with 50k, and user mem lock rlimit is 64k.
2) mlock(addressA, 30k)
3) mlock(addressA, 40k)

Quoted 8 lines

The 3rd step should have been allowed since the 40k request is intersected with the previous 30k at step 2), and the 3rd step is actually for mlock on the extra 10k memory.

This patch checks vma to caculate the actual "new" mlock size, if necessary, and ajust the logic to fix this issue.

Signed-off-by Simon Guo <[email protected]>

Looks reasonable to me. Few nitpicks below.

Quoted 25 lines
---
 mm/mlock.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
mm/mlock.c +13 -0
index 14645be..9283187 100644
@@ -617,6 +617,43 @@ static int apply_vma_lock_flags(unsigned long start, size_t len,
 	return error;
 }
 
+/*
+ * Go through vma areas and sum size of mlocked
+ * vma pages, as return value.
+ * Note deferred memory locking case(mlock2(,,MLOCK_ONFAULT)
+ * is also counted.
+ * Return value: previously mlocked page counts
+ */
+static int count_mm_mlocked_page_nr(struct mm_struct *mm,
+		unsigned long start, size_t len)
+{
+	struct vm_area_struct *vma;
+	int count = 0;
+

mm/mlock.c static int apply_vma_lock_flags(unsigned long start, size_t len, · line 642

Quoted 10 lines
mm/mlock.c +10 -0
+	if (mm == NULL)
+		mm = current->mm;
+
+	vma = find_vma(mm, start);
+	if (vma == NULL)
+		vma = mm->mmap;
+
+	for (; vma ; vma = vma->vm_next) {
+		if (start + len <=  vma->vm_start)
+			break;

	for (; vma && start + len <= vma->vm_start; vma = vma->vm_next) {

mm/mlock.c static int apply_vma_lock_flags(unsigned long start, size_t len, · line 654

Quoted 12 lines
mm/mlock.c +12 -0
+		if (vma->vm_flags && VM_LOCKED) {
+			if (start > vma->vm_start)
+				count -= (start - vma->vm_start);
+			if (start + len < vma->vm_end) {
+				count += start + len - vma->vm_start;
+				break;
+			}
+			count += vma->vm_end - vma->vm_start;
+		}
+	}
+
+	return (PAGE_ALIGN(count) >> PAGE_SHIFT);

Redundant parenthesis.

And do we need PAGE_ALIGN() here? Caller already aligned 'len', and vma boundaries are alinged.

Quoted 32 lines
mm/mlock.c +14 -0
+}
+
 static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
 {
 	unsigned long locked;

@@ -639,6 +676,18 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
 		return -EINTR;
 
 	locked += current->mm->locked_vm;
+	if ((locked > lock_limit) && (!capable(CAP_IPC_LOCK))) {
+		/*
+		 * It is possible that the regions requested
+		 * intersect with previously mlocked areas,
+		 * that part area in "mm->locked_vm" should
+		 * not be counted to new mlock increment
+		 * count. So check and adjust locked count
+		 * if necessary.
+		 */
+		locked -= count_mm_mlocked_page_nr(current->mm,
+				start, len);
+	}
 
 	/* check against resource limits */
 	if ((locked <= lock_limit) || capable(CAP_IPC_LOCK))
Signature · 7 lines
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Signature · 7 lines
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#3 lore raw

PATCH 2/4 mm: mlock: avoid increase mm->locked_vm on mlock() when already mlock2(,MLOCK_ONFAULT)

In reply to [email protected]

From: Simon Guo <[email protected]>

When one vma was with flag VM_LOCKED|VM_LOCKONFAULT (by invoking mlock2(,MLOCK_ONFAULT)), it can again be populated with mlock() with VM_LOCKED flag only.

There is a hole in mlock_fixup() which increase mm->locked_vm twice even the two operations are on the same vma and both with VM_LOCKED flags.

The issue can be reproduced by following code:
mlock2(p, 1024 * 64, MLOCK_ONFAULT); //VM_LOCKED|VM_LOCKONFAULT mlock(p, 1024 * 64); //VM_LOCKED
Then check the increase VmLck field in /proc/pid/status(to 128k).

When vma is set with different vm_flags, and the new vm_flags is with VM_LOCKED, it is not necessarily be a "new locked" vma. This patch corrects this bug by prevent mm->locked_vm from increment when old vm_flags is already VM_LOCKED.

Signed-off-by Simon Guo <[email protected]>
---
 mm/mlock.c | 3 +++
 1 file changed, 3 insertions(+)
mm/mlock.c +3 -0
index 9283187..df29aad 100644
@@ -516,6 +516,7 @@ static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev,
 	int nr_pages;
 	int ret = 0;
 	int lock = !!(newflags & VM_LOCKED);
+	vm_flags_t old_flags = vma->vm_flags;
 
 	if (newflags == vma->vm_flags || (vma->vm_flags & VM_SPECIAL) ||
 	    is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm))

@@ -550,6 +551,8 @@ success:
 	nr_pages = (end - start) >> PAGE_SHIFT;
 	if (!lock)
 		nr_pages = -nr_pages;
+	else if (old_flags & VM_LOCKED)
+		nr_pages = 0;
 	mm->locked_vm += nr_pages;
 
 	/*
Signature · 7 lines
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Acked-by on v1 2/4
#4 lore raw

Re: [PATCH 2/4] mm: mlock: avoid increase mm->locked_vm on mlock() when already mlock2(,MLOCK_ONFAULT)

Quoted 14 lines · On Tue, Aug 30, 2016 at 06:59:39PM +0800, [email protected] wrote:

From: Simon Guo <[email protected]>

When one vma was with flag VM_LOCKED|VM_LOCKONFAULT (by invoking mlock2(,MLOCK_ONFAULT)), it can again be populated with mlock() with VM_LOCKED flag only.

There is a hole in mlock_fixup() which increase mm->locked_vm twice even the two operations are on the same vma and both with VM_LOCKED flags.

The issue can be reproduced by following code:
mlock2(p, 1024 * 64, MLOCK_ONFAULT); //VM_LOCKED|VM_LOCKONFAULT mlock(p, 1024 * 64); //VM_LOCKED
Then check the increase VmLck field in /proc/pid/status(to 128k).

Quoted 6 lines

When vma is set with different vm_flags, and the new vm_flags is with VM_LOCKED, it is not necessarily be a "new locked" vma. This patch corrects this bug by prevent mm->locked_vm from increment when old vm_flags is already VM_LOCKED.

Signed-off-by Simon Guo <[email protected]>
Acked-by Kirill A. Shutemov <[email protected]>
Signature · 7 lines
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#5 lore raw

PATCH 3/4 selftest: split mlock2_ funcs into separate mlock2.h

In reply to [email protected]

From: Simon Guo <[email protected]>

To prepare mlock2.h whose functionality will be reused.

Signed-off-by Simon Guo <[email protected]>
---
 tools/testing/selftests/vm/mlock2-tests.c | 21 +--------------------
 tools/testing/selftests/vm/mlock2.h       | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 20 deletions(-)
 create mode 100644 tools/testing/selftests/vm/mlock2.h
tools/testing/selftests/vm/mlock2-tests.c +1 -20
index 02ca5e0..7cb13ce 100644
@@ -7,27 +7,8 @@
 #include <string.h>
 #include <sys/time.h>
 #include <sys/resource.h>
-#include <syscall.h>
-#include <errno.h>
 #include <stdbool.h>
-
-#ifndef MLOCK_ONFAULT
-#define MLOCK_ONFAULT 1
-#endif
-
-#ifndef MCL_ONFAULT
-#define MCL_ONFAULT (MCL_FUTURE << 1)
-#endif
-
-static int mlock2_(void *start, size_t len, int flags)
-{
-#ifdef __NR_mlock2
-	return syscall(__NR_mlock2, start, len, flags);
-#else
-	errno = ENOSYS;
-	return -1;
-#endif
-}
+#include "mlock2.h"
 
 struct vm_boundaries {
 	unsigned long start;
tools/testing/selftests/vm/mlock2.h new file +20 -0
new file mode 100644
index 0000000..b9c6d9f
@@ -0,0 +1,20 @@
+#include <syscall.h>
+#include <errno.h>
+
+#ifndef MLOCK_ONFAULT
+#define MLOCK_ONFAULT 1
+#endif
+
+#ifndef MCL_ONFAULT
+#define MCL_ONFAULT (MCL_FUTURE << 1)
+#endif
+
+static int mlock2_(void *start, size_t len, int flags)
+{
+#ifdef __NR_mlock2
+	return syscall(__NR_mlock2, start, len, flags);
+#else
+	errno = ENOSYS;
+	return -1;
+#endif
+}
Signature · 7 lines
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#6 lore raw

PATCH 4/4 selftests/vm: add test for mlock() when areas are intersected.

In reply to [email protected]

From: Simon Guo <[email protected]>

This patch adds mlock() test for multiple invocation on the same address area, and verify it doesn't mess the rlimit mlock limitation.

Signed-off-by Simon Guo <[email protected]>
---
 tools/testing/selftests/vm/.gitignore             |  1 +
 tools/testing/selftests/vm/Makefile               |  4 ++
 tools/testing/selftests/vm/mlock-intersect-test.c | 76 +++++++++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 tools/testing/selftests/vm/mlock-intersect-test.c
tools/testing/selftests/vm/.gitignore +1 -0
index a937a9d..142c565 100644
@@ -7,3 +7,4 @@ mlock2-tests
 on-fault-limit
 transhuge-stress
 userfaultfd
+mlock-intersect-test
tools/testing/selftests/vm/Makefile +4 -0
index e4bb1de..a0412a8 100644
@@ -10,6 +10,7 @@ BINARIES += on-fault-limit
 BINARIES += thuge-gen
 BINARIES += transhuge-stress
 BINARIES += userfaultfd
+BINARIES += mlock-intersect-test
 
 all: $(BINARIES)
 %: %.c

@@ -17,6 +18,9 @@ all: $(BINARIES)
 userfaultfd: userfaultfd.c ../../../../usr/include/linux/kernel.h
 	$(CC) $(CFLAGS) -O2 -o $@ $< -lpthread
 
+mlock-intersect-test: mlock-intersect-test.c
+	$(CC) $(CFLAGS) -o $@ $< -lcap
+
 ../../../../usr/include/linux/kernel.h:
 	make -C ../../../.. headers_install
tools/testing/selftests/vm/mlock-intersect-test.c new file +76 -0

76 lines not on this page · raw message

Signature · 7 lines
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#7 lore raw

Re: [PATCH 4/4] selftests/vm: add test for mlock() when areas are intersected.

Quoted 6 lines · On Tue, 30 Aug 2016, [email protected] wrote:

From: Simon Guo <[email protected]>

This patch adds mlock() test for multiple invocation on the same address area, and verify it doesn't mess the rlimit mlock limitation.

Thanks for expanding mlock testing. I'm wondering if you are interested in more elaborate testing that doesn't only check what you are fixing, such as mlock(p + x, 40k) where x is < 20k?

Would you also be willing to make sure that the rlimit is actually enforced when it's expected to?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#8 lore raw

Re: [PATCH 4/4] selftests/vm: add test for mlock() when areas are intersected.

Hi David,

Quoted 9 lines · David Rientjes
Quoted 6 lines · On Tue, 30 Aug 2016, [email protected] wrote:

From: Simon Guo <[email protected]>

This patch adds mlock() test for multiple invocation on the same address area, and verify it doesn't mess the rlimit mlock limitation.

Quoted 6 lines

Thanks for expanding mlock testing. I'm wondering if you are interested in more elaborate testing that doesn't only check what you are fixing, such as mlock(p + x, 40k) where x is < 20k?

Would you also be willing to make sure that the rlimit is actually enforced when it's expected to?

I'd like to do so.
Let me think more for the comprehensive testing. If you have any other test cases in mind, please let me know.

Thanks,
- Simon

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>