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 <[email protected] <javascript:>> 
> --- 
>  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 [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to