From: Stephen Hemminger <stephen@networkplumber.org>
To: Fengnan Chang <changfengnan@bytedance.com>
Cc: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>,
anatoly.burakov@intel.com, dev@dpdk.org, xuemingl@mellanox.com
Subject: Re: [External] Re: [PATCH] eal: fix modify data area after memset
Date: Wed, 25 Oct 2023 09:03:57 -0700 [thread overview]
Message-ID: <20231025090357.60c1f56e@hermes.local> (raw)
In-Reply-To: <CAPFOzZuX--9p-dO69C7DiL=R8Z=iU3mg3Jn31LXPHRxt8O9gvQ@mail.gmail.com>
On Mon, 23 Oct 2023 17:07:21 +0800
Fengnan Chang <changfengnan@bytedance.com> wrote:
> Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> 于2023年10月23日周一 04:22写道：
> >
> > 2023-09-22 16:12 (UTC+0800), Fengnan Chang:
> > > ping
> > >
> > > Fengnan Chang <changfengnan@bytedance.com> 于2023年9月12日周二 17:05写道：
> > > >
> > > > Let's look at this path:
> > > > malloc_elem_free
> > > > ->malloc_elem_join_adjacent_free
> > > > ->join_elem(elem, elem->next)
> > > >
> > > > 0. cur elem's pad > 0
> > > > 1. data area memset in malloc_elem_free first.
> > > > 2. next elem is free, try to join cur elem and next.
> > > > 3. in join_elem, try to modify inner->size, this address had
> > > > memset in step 1, it casue the content of addrees become non-zero.
> > > >
> > > > If user call rte_zmalloc, and pick this elem, it can't get all
> > > > zero'd memory.
> >
> > malloc_elem_join_adjacent_free() always calls memset() after join_elem(),
> > for the next and the previous element respectively.
> when try to call join_elem() for the next element in
> malloc_elem_join_adjacent_free(),
> the memset is try to memset *next* element, but join_elem() is update
> *current* element's
> content, which shoudn't happen, it's two different element.
>
> > How to reproduce this bug?
> when I test this patch,
> https://patches.dpdk.org/project/dpdk/patch/20230831111937.60975-1-changfengnan@bytedance.com/
> I have a case try to alloc 64/128/192 size object and free with 16 threads,
> after every
> alloc I'll check wheather all content is 0 or not.
> It's not easy to reproduce, you can have a try, it's easier to find
> this problem in code level.
I tried to make a test that would reproduce the problem but it did not.
diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index cd579c503cf5..cfd45d6a28eb 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -28,6 +28,7 @@
#include <rte_string_fns.h>
#define N 10000
+#define BINS 100
static int
is_mem_on_socket(int32_t socket);
@@ -69,13 +70,24 @@ is_aligned(void *p, int align)
return 1;
}
+static bool is_all_zero(uint8_t *mem, size_t sz)
+{
+ size_t i;
+
+ for (i = 0; i < sz; i++)
+ if (mem[i] != 0)
+ return false;
+
+ return true;
+}
+
static int
test_align_overlap_per_lcore(__rte_unused void *arg)
{
const unsigned align1 = 8,
align2 = 64,
align3 = 2048;
- unsigned i,j;
+ unsigned int i;
void *p1 = NULL, *p2 = NULL, *p3 = NULL;
int ret = 0;
@@ -86,11 +98,12 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
ret = -1;
break;
}
- for(j = 0; j < 1000 ; j++) {
- if( *(char *)p1 != 0) {
- printf("rte_zmalloc didn't zero the allocated memory\n");
- ret = -1;
- }
+
+ if (!is_all_zero(p1, 1000)) {
+ printf("rte_zmalloc didn't zero the allocated memory\n");
+ ret = -1;
+ rte_free(p1);
+ break;
}
p2 = rte_malloc("dummy", 1000, align2);
if (!p2){
@@ -140,6 +153,66 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
return ret;
}
+/*
+ * Allocate random size chunks and make sure that they are
+ * always zero.
+ */
+static int
+test_zmalloc(__rte_unused void *arg)
+{
+ unsigned int i, n;
+ void *slots[BINS] = { };
+ void *p1;
+ size_t sz;
+
+ /* Allocate many variable size chunks */
+ for (i = 0; i < BINS; i++) {
+ sz = rte_rand_max(1024) + 1;
+ p1 = rte_zmalloc("slots", sz, 0);
+ if (p1 == NULL) {
+ printf("rte_zmalloc(%zu) returned NULL (i=%u)\n", sz, i);
+ goto fail;
+ }
+ slots[i] = p1;
+ if (!is_all_zero(p1, sz))
+ goto fail;
+ }
+
+ /* Drop one chunk per iteration */
+ for (n = BINS; n > 0; n--) {
+ /* Swap in a new block into a slot */
+ for (i = 0; i < N; i++) {
+ unsigned int bin = rte_rand_max(n);
+
+ sz = rte_rand_max(1024) + 1;
+ p1 = rte_zmalloc("swap", sz, 0);
+ if (!p1){
+ printf("rte_zmalloc(%zu) returned NULL (i=%u)\n", sz, i);
+ goto fail;
+ }
+
+ if (!is_all_zero(p1, sz)) {
+ printf("rte_zmalloc didn't zero the allocated memory\n");
+ goto fail;
+ }
+
+ rte_free(slots[bin]);
+ slots[bin] = p1;
+ }
+
+ /* Drop last bin */
+ rte_free(slots[n]);
+ slots[n] = NULL;
+ }
+
+ return 0;
+fail:
+ for (i = 0; i < BINS; i++)
+ rte_free(slots[i]);
+
+ return -1;
+}
+
static int
test_reordered_free_per_lcore(__rte_unused void *arg)
{
@@ -1020,6 +1091,21 @@ test_malloc(void)
}
else printf("test_realloc() passed\n");
+ /*----------------------------*/
+ RTE_LCORE_FOREACH_WORKER(lcore_id) {
+ rte_eal_remote_launch(test_zmalloc, NULL, lcore_id);
+ }
+
+ RTE_LCORE_FOREACH_WORKER(lcore_id) {
+ if (rte_eal_wait_lcore(lcore_id) < 0)
+ ret = -1;
+ }
+ if (ret < 0){
+ printf("test_zmalloc() failed\n");
+ return ret;
+ }
+ else printf("test_zmalloc() passed\n");
+
/*----------------------------*/
RTE_LCORE_FOREACH_WORKER(lcore_id) {
rte_eal_remote_launch(test_align_overlap_per_lcore, NULL, lcore_id);
next prev parent reply other threads:[~2023-10-25 16:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-12 9:04 Fengnan Chang
2023-09-22 8:12 ` Fengnan Chang
2023-10-22 20:22 ` Dmitry Kozlyuk
2023-10-23 9:07 ` [External] " Fengnan Chang
2023-10-25 16:03 ` Stephen Hemminger [this message]
2023-10-30 12:31 ` Fengnan Chang
2023-10-17 13:32 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231025090357.60c1f56e@hermes.local \
--to=stephen@networkplumber.org \
--cc=anatoly.burakov@intel.com \
--cc=changfengnan@bytedance.com \
--cc=dev@dpdk.org \
--cc=dmitry.kozliuk@gmail.com \
--cc=xuemingl@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).