[dpdk-dev] eal: fix end for bounded malloc elements

Message ID be459294da93f5013e8a7be203a0253787224b2d.1513865614.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Anatoly Burakov Dec. 21, 2017, 4:54 p.m. UTC
  In cases when alignment is bigger than boundary, we may incorrectly
calculate end of a bounded malloc element.

Consider this: suppose we are allocating a bounded malloc element
that should be of 128 bytes in size, bounded to 128 bytes and
aligned on a 256-byte boundary. Suppose our malloc element ends
at 0x140 - that is, 256 plus one cacheline.

So, right at the start, we are aligning our new_data_start to
include the required element size, and to be aligned on a specified
boundary - so new_data_start becomes 0. This fails the following
bounds check, because our element cannot go above 128 bytes from
the start, and we are at 320. So, we enter the bounds handling
branch.

While we're in there, we are aligning end_pt to our boundedness
requirement of 128 byte, and end up with 0x100 (since 256 is
128-byte aligned). We recalculate new_data_size and it stays at
0, however our end is at 0x100, which is beyond the 128 byte
boundary, and we report inability to reserve a bounded element
when we could have.

This patch adds an end_pt recalculation after new_data_start
adjustment - we already know that size <= bound, so we can do it
safely - and we then correctly report that we can, in fact, try
using this element for bounded malloc allocation.

Fixes: fafcc11985a2 ("mem: rework memzone to be allocated by malloc")
Cc: sergio.gonzalez.monroy@intel.com
Cc: stable@dpdk.org
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/malloc_elem.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Thomas Monjalon Jan. 12, 2018, 2:54 p.m. UTC | #1
21/12/2017 17:54, Anatoly Burakov:
> In cases when alignment is bigger than boundary, we may incorrectly
> calculate end of a bounded malloc element.
> 
> Consider this: suppose we are allocating a bounded malloc element
> that should be of 128 bytes in size, bounded to 128 bytes and
> aligned on a 256-byte boundary. Suppose our malloc element ends
> at 0x140 - that is, 256 plus one cacheline.
> 
> So, right at the start, we are aligning our new_data_start to
> include the required element size, and to be aligned on a specified
> boundary - so new_data_start becomes 0. This fails the following
> bounds check, because our element cannot go above 128 bytes from
> the start, and we are at 320. So, we enter the bounds handling
> branch.
> 
> While we're in there, we are aligning end_pt to our boundedness
> requirement of 128 byte, and end up with 0x100 (since 256 is
> 128-byte aligned). We recalculate new_data_size and it stays at
> 0, however our end is at 0x100, which is beyond the 128 byte
> boundary, and we report inability to reserve a bounded element
> when we could have.
> 
> This patch adds an end_pt recalculation after new_data_start
> adjustment - we already know that size <= bound, so we can do it
> safely - and we then correctly report that we can, in fact, try
> using this element for bounded malloc allocation.
> 
> Fixes: fafcc11985a2 ("mem: rework memzone to be allocated by malloc")
> Cc: sergio.gonzalez.monroy@intel.com
> Cc: stable@dpdk.org
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

It looks to be a headache, but as the maintainer of DPDK memory,
I trust you :)

Applied, thanks
  

Patch

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index 98bcd37..f6cbc42 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -98,6 +98,7 @@  elem_start_pt(struct malloc_elem *elem, size_t size, unsigned align,
 	if ((new_data_start & bmask) != ((end_pt - 1) & bmask)) {
 		end_pt = RTE_ALIGN_FLOOR(end_pt, bound);
 		new_data_start = RTE_ALIGN_FLOOR((end_pt - size), align);
+		end_pt = new_data_start + size;
 		if (((end_pt - 1) & bmask) != (new_data_start & bmask))
 			return NULL;
 	}