Skip to content
Permalink
Browse files

lib/os/mempool: Fix corruption case with block splitting

The block_fits() predicate was borked.  It would check that a block
fits within the bounds of the whole heap.  But that's not enough:
because of alignment changes between levels the sub-blocks may be
adjusted forward.  It needs to fit inside the PARENT block that it was
split from.

What could happen at runtime is that the last subblocks of a
misaligned parent block would overlap memory from subsequent blocks,
or even run off the end of the heap.  That's bad.

Change the API of block_fits() a little so it can extract the parent
region and do this properly.

Fixes #15279.  Passes test introduced in #16728 to demonstrate what
seems like the same issue.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
  • Loading branch information...
andyross authored and andrewboie committed Jun 20, 2019
1 parent 8482218 commit d0490fe9f96e7e1f2358f2ced29c0ed458bdbb99
Showing with 19 additions and 5 deletions.
  1. +19 −5 lib/os/mempool.c
@@ -72,9 +72,23 @@ static size_t buf_size(struct sys_mem_pool_base *p)
return p->n_max * p->max_sz;
}

static bool block_fits(struct sys_mem_pool_base *p, void *block, size_t bsz)
static bool block_fits(struct sys_mem_pool_base *p,
int lvl, int bn, size_t *lsizes)
{
return ((u8_t *)block + bsz - 1 - (u8_t *)p->buf) < buf_size(p);
u8_t *parent, *block_end;
size_t parent_sz;

block_end = (u8_t *)block_ptr(p, lsizes[lvl], bn) + lsizes[lvl];

if (lvl == 0) {
parent_sz = buf_size(p);
parent = p->buf;
} else {
parent_sz = lsizes[lvl - 1];
parent = block_ptr(p, lsizes[lvl - 1], bn / 4);
}

return block_end <= (parent + parent_sz);
}

void z_sys_mem_pool_base_init(struct sys_mem_pool_base *p)
@@ -161,7 +175,7 @@ static unsigned int bfree_recombine(struct sys_mem_pool_base *p, int level,
int i, lsz = lsizes[level];
void *block = block_ptr(p, lsz, bn);

__ASSERT(block_fits(p, block, lsz), "");
__ASSERT(block_fits(p, level, bn, lsizes), "");

/* Put it back */
set_free_bit(p, level, bn);
@@ -179,7 +193,7 @@ static unsigned int bfree_recombine(struct sys_mem_pool_base *p, int level,
for (i = 0; i < 4; i++) {
int b = (bn & ~3) + i;

if (block_fits(p, block_ptr(p, lsz, b), lsz)) {
if (block_fits(p, level, b, lsizes)) {
clear_free_bit(p, level, b);
sys_dlist_remove(block_ptr(p, lsz, b));
}
@@ -220,7 +234,7 @@ static void *block_break(struct sys_mem_pool_base *p, void *block, int l,
void *block2 = (lsz * i) + (char *)block;

set_free_bit(p, l + 1, lbn);
if (block_fits(p, block2, lsz)) {
if (block_fits(p, l + 1, lbn, lsizes)) {
sys_dlist_append(&p->levels[l + 1].free_list, block2);
}
}

0 comments on commit d0490fe

Please sign in to comment.
You can’t perform that action at this time.