diff mbox series

malloc: don't skip pad on free

Message ID 038143a314345e9c5bf76b1287497a5c4c9f63ed.1531992860.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series malloc: don't skip pad on free |

Checks

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

Commit Message

Burakov, Anatoly July 19, 2018, 9:42 a.m. UTC 
  Previously, we were skipping erasing pad because we were
expecting it to be freed when we were merging adjacent
segments. However, if there were no adjacent segments to
merge, we would've skipped erasing the pad, leaving non-zero
memory in our free space.

Fix this by including pad in the erasing unconditionally.

Fixes: e43a9f52b7ff ("malloc: fix pad erasing")
Cc: stable@dpdk.org

Reported-by: Andrew Rybchenko <arybchenko@solarflare.com>

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    I have confirmed the issue with unit tests - adding a simple zero-check
    function on alloc will throw errors when running malloc_autotest on
    latest master, but the errors go away once this patch is applied.
    
    Our unit test's zmalloc calls check if memory is not zero, but this
    condition is rare enough not to be triggered by it, and regular
    malloc calls aren't checked on zeroed out memory. The bulk of the
    malloc calls in the unit tests are malloc, not zmalloc, so pretty
    much all of the time the memory is not checked for being zero on
    alloc.

 lib/librte_eal/common/malloc_elem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andrew Rybchenko July 19, 2018, 4:37 p.m. UTC | #1 
On 19.07.2018 12:42, Anatoly Burakov wrote:
> Previously, we were skipping erasing pad because we were
> expecting it to be freed when we were merging adjacent
> segments. However, if there were no adjacent segments to
> merge, we would've skipped erasing the pad, leaving non-zero
> memory in our free space.
>
> Fix this by including pad in the erasing unconditionally.
>
> Fixes: e43a9f52b7ff ("malloc: fix pad erasing")
> Cc: stable@dpdk.org
>
> Reported-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Many thanks, the patch fixes the problem I've observed.

Tested-by: Andrew Rybchenko <arybchenko@solarflare.com>
Thomas Monjalon July 20, 2018, 9:27 a.m. UTC | #2 
19/07/2018 18:37, Andrew Rybchenko:
> On 19.07.2018 12:42, Anatoly Burakov wrote:
> > Previously, we were skipping erasing pad because we were
> > expecting it to be freed when we were merging adjacent
> > segments. However, if there were no adjacent segments to
> > merge, we would've skipped erasing the pad, leaving non-zero
> > memory in our free space.
> >
> > Fix this by including pad in the erasing unconditionally.
> >
> > Fixes: e43a9f52b7ff ("malloc: fix pad erasing")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Many thanks, the patch fixes the problem I've observed.
> 
> Tested-by: Andrew Rybchenko <arybchenko@solarflare.com>

Applied, thanks
diff mbox series

Patch

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index efcb82677..e0a8ed15b 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -519,8 +519,8 @@  malloc_elem_free(struct malloc_elem *elem)
 	void *ptr;
 	size_t data_len;
 
-	ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN + elem->pad);
-	data_len = elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
+	ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN);
+	data_len = elem->size - MALLOC_ELEM_OVERHEAD;
 
 	elem = malloc_elem_join_adjacent_free(elem);