So, it is possible to reproduce the issue with a unit test, however it requires 
specifying an explicit memory size to the run script.

The issue only occurs when the bitset size is an exact multiple of 
mmu::huge_page_size.  In that case, the first mmap that is greater than or 
equal to mmu::huge_page_size will be allocated from the top most range.
When that allocation is munmapped, the system will hang.

Please see the patch below which adds a tst-page-range-allocator.cc unit test.

After applying and rebuilding the test image you should be able to trigger the 
hang by calling:

timmons@rtp-skink osv[0]> scripts/run.py -m512.15M -c2 -e 
tests/tst-page-range-allocator.so
warning: TCG doesn't support requested feature: CPUID.01H:ECX.vmx [bit 5]
warning: TCG doesn't support requested feature: CPUID.01H:ECX.vmx [bit 5]
OSv v0.24-474-g9cceaca
eth0: 192.168.122.15
_bitmap.size() = 131072, end = 131072
[ hangs here… ]

P.S.  Since the unit test requires an explicit configuration, I’m not sure if 
it’s useful to add it to the patch… Let me know if you think otherwise.


diff --git a/core/mempool.cc b/core/mempool.cc
index 569a0ac..437b54c 100644
--- a/core/mempool.cc
+++ b/core/mempool.cc
@@ -767,6 +767,10 @@ void page_range_allocator::free(page_range* pr)
         pr2->size += pr->size;
         pr = pr2;
     }
+    auto end = get_bitmap_idx(*pr) + pr->size / page_size;
+    if (end >= _bitmap.size()) {
+        printf("_bitmap.size() = %zu, end = %zu\n", _bitmap.size(), end);
+    }
     if (_bitmap[get_bitmap_idx(*pr) + pr->size / page_size]) {
         auto pr2 = static_cast<page_range*>(static_cast<void*>(pr) + pr->size);
         remove(*pr2);
diff --git a/modules/tests/Makefile b/modules/tests/Makefile
index b8348dd..715a7c1 100644
--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -87,7 +87,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so 
tst-bsd-evh.so \
        tst-ifaddrs.so tst-pthread-affinity-inherit.so tst-sem-timed-wait.so \
        tst-ttyname.so tst-pthread-barrier.so tst-feexcept.so tst-math.so \
        tst-sigaltstack.so tst-fread.so tst-tcp-cork.so tst-tcp-v6.so \
-       tst-calloc.so
+       tst-calloc.so tst-page-range-allocator.so
 
 #      libstatic-thread-variable.so tst-static-thread-variable.so \
 
diff --git a/tests/tst-page-range-allocator.cc 
b/tests/tst-page-range-allocator.cc
new file mode 100644
index 0000000..bf2a9aa
--- /dev/null
+++ b/tests/tst-page-range-allocator.cc
@@ -0,0 +1,28 @@
+#include <osv/mmu.hh>
+#include <sys/mman.h>
+#include <string.h>
+
+int tests = 0, fails = 0;
+
+constexpr int bufsize = mmu::huge_page_size;
+
+static void report(bool ok, const char *msg)
+{
+    ++tests;
+    fails += !ok;
+    debug("%s: %s\n", (ok ? "PASS" : "FAIL"), msg);
+}
+
+int main()
+{
+    void *buffer = mmap(NULL, bufsize, PROT_READ|PROT_WRITE, 
MAP_SHARED|MAP_ANONYMOUS, -1, 0);
+    report(buffer != nullptr, "mmap");
+
+    // Force the buffer to get mapped.
+    memset(buffer, 0, bufsize);
+
+    int error = munmap(buffer, bufsize);
+    report(error == 0, "mumap");
+
+    debug("SUMMARY: %d tests, %d failures\n", tests, fails);
+}

From: OSv Gopher <osv-dev@googlegroups.com> on behalf of Timmons Player 
<timmons.pla...@spirent.com>
Date: Monday, January 15, 2018 at 3:05 PM
To: Waldek Kozaczuk <jwkozac...@gmail.com>, OSv Gopher 
<osv-dev@googlegroups.com>
Subject: Re: [PATCH] memory: respect _bitset length bounds in 
page_range_allocator

I don’t know.  I could only ever reproduce it on 1 CPU setups on a specific 
hypervisor.  I’ll investigate reproducing it via a unit test.
 
Timmons
 
 
From: OSv Gopher <osv-dev@googlegroups.com> on behalf of Waldek Kozaczuk 
<jwkozac...@gmail.com>
Date: Monday, January 15, 2018 at 2:19 PM
To: OSv Gopher <osv-dev@googlegroups.com>
Subject: Re: [PATCH] memory: respect _bitset length bounds in 
page_range_allocator
 
How difficult would it be to create a unit test around this bug to prove what 
it fixes?

On Monday, January 15, 2018 at 12:52:24 PM UTC-5, Timmons C. Player wrote: 
The page_range_allocator attempts to merge neighbor page ranges 
when a page range is freed in order to minimize memory fragmentation. 

However, the original implementation didn't validate indexes before 
checking the _bitset to determine whether a page range had a neighbor 
or not.  This change make sure that indexes stay within bounds. 

This is the correct fix for the fix attempted in 
44f8324e3f2e53f55022a1d93c8b072514e7ac07 

Signed-off-by: Timmons C. Player <timmons...@spirent.com> 
--- 
 core/mempool.cc | 6 ++++-- 
 1 file changed, 4 insertions(+), 2 deletions(-) 

diff --git a/core/mempool.cc b/core/mempool.cc 
index e0d0867..1ac3f67 100644 
--- a/core/mempool.cc 
+++ b/core/mempool.cc 
@@ -761,13 +761,15 @@ page_range* page_range_allocator::alloc_aligned(size_t 
size, size_t offset, 
  
 void page_range_allocator::free(page_range* pr) 
 { 
-    if (_bitmap[get_bitmap_idx(*pr) - 1]) { 
+    auto idx = get_bitmap_idx(*pr); 
+    if (idx && _bitmap[idx - 1]) { 
         auto pr2 = *(reinterpret_cast<page_range**>(pr) - 1); 
         remove(*pr2); 
         pr2->size += pr->size; 
         pr = pr2; 
     } 
-    if (_bitmap[get_bitmap_idx(*pr) + pr->size / page_size - 1]) { 
+    auto next_idx = get_bitmap_idx(*pr) + pr->size / page_size; 
+    if (next_idx < _bitmap.size() && _bitmap[next_idx]) { 
         auto pr2 = static_cast<page_range*>(static_cast<void*>(pr) + 
pr->size); 
         remove(*pr2); 
         pr->size += pr2->size; 
-- 
2.7.4 
-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.





Spirent Communications e-mail confidentiality.
------------------------------------------------------------------------
This e-mail contains confidential and / or privileged information belonging to 
Spirent Communications plc, its affiliates and / or subsidiaries. If you are 
not the intended recipient, you are hereby notified that any disclosure, 
copying, distribution and / or the taking of any action based upon reliance on 
the contents of this transmission is strictly forbidden. If you have received 
this message in error please notify the sender by return e-mail and delete it 
from your system.

Spirent Communications plc
Northwood Park, Gatwick Road, Crawley, West Sussex, RH10 9XN, United Kingdom.
Tel No. +44 (0) 1293 767676
Fax No. +44 (0) 1293 767677

Registered in England Number 470893
Registered at Northwood Park, Gatwick Road, Crawley, West Sussex, RH10 9XN, 
United Kingdom.

Or if within the US, 

Spirent Communications,
27349 Agoura Road, Calabasas, CA, 91301, USA.
Tel No. 1-818-676- 2300 
-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to