Segmentation fault may occur without checking if memzone reserves succeed or not. This patch fixed it. Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") Cc: stable@dpdk.org Signed-off-by: Min Hu (Connor) <humin29@huawei.com> --- app/test/test_timer_secondary.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/test/test_timer_secondary.c b/app/test/test_timer_secondary.c index 1e8f1d4..281f5bd 100644 --- a/app/test/test_timer_secondary.c +++ b/app/test/test_timer_secondary.c @@ -125,6 +125,11 @@ test_timer_secondary(void) mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, sizeof(*test_info), SOCKET_ID_ANY, 0); + if (mz == NULL) { + printf("Failed to reserve memzone\n"); + return TEST_SKIPPED; + } + test_info = mz->addr; TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for " "test data"); @@ -171,6 +176,11 @@ test_timer_secondary(void) int i; mz = rte_memzone_lookup(TEST_INFO_MZ_NAME); + if (mz == NULL) { + printf("Failed to lookup memzone\n"); + return TEST_SKIPPED; + } + test_info = mz->addr; TEST_ASSERT_NOT_NULL(test_info, "Couldn't lookup memzone for " "test info"); -- 2.7.4
> -----Original Message----- > From: Min Hu (Connor) <humin29@huawei.com> > Sent: Thursday, April 22, 2021 4:19 AM > To: dev@dpdk.org > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; rsanford@akamai.com; Carrillo, > Erik G <erik.g.carrillo@intel.com> > Subject: [PATCH] test/timer: fix memzone reserve failure check > > Segmentation fault may occur without checking if memzone reserves > succeed or not. > > This patch fixed it. > > Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") > Cc: stable@dpdk.org > > Signed-off-by: Min Hu (Connor) <humin29@huawei.com> > --- > app/test/test_timer_secondary.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/app/test/test_timer_secondary.c > b/app/test/test_timer_secondary.c index 1e8f1d4..281f5bd 100644 > --- a/app/test/test_timer_secondary.c > +++ b/app/test/test_timer_secondary.c > @@ -125,6 +125,11 @@ test_timer_secondary(void) > > mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, > sizeof(*test_info), > SOCKET_ID_ANY, 0); > + if (mz == NULL) { > + printf("Failed to reserve memzone\n"); > + return TEST_SKIPPED; > + } > + > test_info = mz->addr; > TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate > memory for " > "test data"); I think the TEST_ASSERT_NOT_NULL check here should be the area we update -- instead of checking the test_info pointer, we should check "mz", and that will address the issue you have noted. We can then move the "test_info = mz->addr" assignment below the assert. > @@ -171,6 +176,11 @@ test_timer_secondary(void) > int i; > > mz = rte_memzone_lookup(TEST_INFO_MZ_NAME); > + if (mz == NULL) { > + printf("Failed to lookup memzone\n"); > + return TEST_SKIPPED; > + } > + > test_info = mz->addr; > TEST_ASSERT_NOT_NULL(test_info, "Couldn't lookup > memzone for " > "test info"); Same thing here -- we can update the TEST_ASSERT_NOT_NULL call here instead, and move it above the "test_info = mz->addr" assignment. Thanks, Erik > -- > 2.7.4
Segmentation fault may occur without checking if memzone reserves succeed or not. This patch fixed it. Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") Cc: stable@dpdk.org Signed-off-by: Min Hu (Connor) <humin29@huawei.com> --- v2: * use TEST_ASSERT_NOT_NULL check "mz" instead of checking the test_info pointer. --- app/test/test_timer_secondary.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/test/test_timer_secondary.c b/app/test/test_timer_secondary.c index 1e8f1d4..16a9f18 100644 --- a/app/test/test_timer_secondary.c +++ b/app/test/test_timer_secondary.c @@ -125,9 +125,9 @@ test_timer_secondary(void) mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, sizeof(*test_info), SOCKET_ID_ANY, 0); - test_info = mz->addr; - TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for " + TEST_ASSERT_NOT_NULL(mz, "Couldn't allocate memory for " "test data"); + test_info = mz->addr; test_info->tim_mempool = rte_mempool_create("test_timer_mp", NUM_TIMERS, sizeof(struct rte_timer), 0, 0, @@ -171,9 +171,9 @@ test_timer_secondary(void) int i; mz = rte_memzone_lookup(TEST_INFO_MZ_NAME); - test_info = mz->addr; - TEST_ASSERT_NOT_NULL(test_info, "Couldn't lookup memzone for " + TEST_ASSERT_NOT_NULL(mz, "Couldn't lookup memzone for " "test info"); + test_info = mz->addr; for (i = 0; i < NUM_TIMERS; i++) { rte_mempool_get(test_info->tim_mempool, (void **)&tim); -- 2.7.4
在 2021/5/2 4:00, Carrillo, Erik G 写道: >> -----Original Message----- >> From: Min Hu (Connor) <humin29@huawei.com> >> Sent: Thursday, April 22, 2021 4:19 AM >> To: dev@dpdk.org >> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; rsanford@akamai.com; Carrillo, >> Erik G <erik.g.carrillo@intel.com> >> Subject: [PATCH] test/timer: fix memzone reserve failure check >> >> Segmentation fault may occur without checking if memzone reserves >> succeed or not. >> >> This patch fixed it. >> >> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") >> Cc: stable@dpdk.org >> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com> >> --- >> app/test/test_timer_secondary.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/app/test/test_timer_secondary.c >> b/app/test/test_timer_secondary.c index 1e8f1d4..281f5bd 100644 >> --- a/app/test/test_timer_secondary.c >> +++ b/app/test/test_timer_secondary.c >> @@ -125,6 +125,11 @@ test_timer_secondary(void) >> >> mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, >> sizeof(*test_info), >> SOCKET_ID_ANY, 0); >> + if (mz == NULL) { >> + printf("Failed to reserve memzone\n"); >> + return TEST_SKIPPED; >> + } >> + >> test_info = mz->addr; >> TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate >> memory for " >> "test data"); > > I think the TEST_ASSERT_NOT_NULL check here should be the area we update -- instead of checking the test_info pointer, we should check "mz", and that will address the issue you have noted. We can then move the "test_info = mz->addr" assignment below the assert. > Agreed, fixed in v2, thanks. >> @@ -171,6 +176,11 @@ test_timer_secondary(void) >> int i; >> >> mz = rte_memzone_lookup(TEST_INFO_MZ_NAME); >> + if (mz == NULL) { >> + printf("Failed to lookup memzone\n"); >> + return TEST_SKIPPED; >> + } >> + >> test_info = mz->addr; >> TEST_ASSERT_NOT_NULL(test_info, "Couldn't lookup >> memzone for " >> "test info"); > > Same thing here -- we can update the TEST_ASSERT_NOT_NULL call here instead, and move it above the "test_info = mz->addr" assignment. > Fixed in v2, thanks. > Thanks, > Erik > >> -- >> 2.7.4 > > . >
> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Monday, May 3, 2021 8:08 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Carrillo, Erik G
> <erik.g.carrillo@intel.com>; rsanford@akamai.com
> Subject: [PATCH v2] test/timer: fix memzone reserve failure check
>
> Segmentation fault may occur without checking if memzone reserves
> succeed or not.
>
> This patch fixed it.
>
> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")
> Cc: stable@dpdk.org
>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v2:
> * use TEST_ASSERT_NOT_NULL check "mz" instead of checking
> the test_info pointer.
> ---
Thanks,
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
04/05/2021 03:07, Min Hu (Connor): > Segmentation fault may occur without checking if memzone > reserves succeed or not. The sentence is confusing. Please try to make it more logical. Something like "It was potentially dereferencing a null pointer. It is fixed by checking the pointer before dereferencing." > > This patch fixed it. > > Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") > Cc: stable@dpdk.org > > Signed-off-by: Min Hu (Connor) <humin29@huawei.com> [...] > - test_info = mz->addr; > - TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for " > + TEST_ASSERT_NOT_NULL(mz, "Couldn't allocate memory for " > "test data"); Error messages should not be split. I makes search difficult. Please fix it in this patch while touching this line.
Segmentation fault may occur without checking if memzone reserves succeed or not. This patch fixed it. Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") Cc: stable@dpdk.org Signed-off-by: Min Hu (Connor) <humin29@huawei.com> --- v3: * do not split error messages. v2: * use TEST_ASSERT_NOT_NULL check "mz" instead of checking the test_info pointer. --- app/test/test_timer_secondary.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/test/test_timer_secondary.c b/app/test/test_timer_secondary.c index 1e8f1d4..decfaef 100644 --- a/app/test/test_timer_secondary.c +++ b/app/test/test_timer_secondary.c @@ -125,9 +125,8 @@ test_timer_secondary(void) mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, sizeof(*test_info), SOCKET_ID_ANY, 0); + TEST_ASSERT_NOT_NULL(mz, "Couldn't allocate memory for test data"); test_info = mz->addr; - TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for " - "test data"); test_info->tim_mempool = rte_mempool_create("test_timer_mp", NUM_TIMERS, sizeof(struct rte_timer), 0, 0, @@ -171,9 +170,8 @@ test_timer_secondary(void) int i; mz = rte_memzone_lookup(TEST_INFO_MZ_NAME); + TEST_ASSERT_NOT_NULL(mz, "Couldn't lookup memzone for test info"); test_info = mz->addr; - TEST_ASSERT_NOT_NULL(test_info, "Couldn't lookup memzone for " - "test info"); for (i = 0; i < NUM_TIMERS; i++) { rte_mempool_get(test_info->tim_mempool, (void **)&tim); -- 2.7.4
在 2021/5/10 22:13, Thomas Monjalon 写道: > 04/05/2021 03:07, Min Hu (Connor): >> Segmentation fault may occur without checking if memzone >> reserves succeed or not. > > The sentence is confusing. Please try to make it more logical. > Something like "It was potentially dereferencing a null pointer. > It is fixed by checking the pointer before dereferencing." > >> >> This patch fixed it. >> >> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") >> Cc: stable@dpdk.org >> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com> > [...] >> - test_info = mz->addr; >> - TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for " >> + TEST_ASSERT_NOT_NULL(mz, "Couldn't allocate memory for " >> "test data"); > > Error messages should not be split. I makes search difficult. > Please fix it in this patch while touching this line. > Fixed in v3, thanks. > > > . >
11/05/2021 02:53, Min Hu (Connor):
> Segmentation fault may occur without checking if memzone
> reserves succeed or not.
>
> This patch fixed it.
>
> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")
> Cc: stable@dpdk.org
>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
You missed reporting previous ack:
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Applied, thanks
PS: please do not forget marking old patches as superseded
when sending a new version.
在 2021/5/12 22:36, Thomas Monjalon 写道: > 11/05/2021 02:53, Min Hu (Connor): >> Segmentation fault may occur without checking if memzone >> reserves succeed or not. >> >> This patch fixed it. >> >> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process") >> Cc: stable@dpdk.org >> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com> > > You missed reporting previous ack: > Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com> > > Applied, thanks > > PS: please do not forget marking old patches as superseded > when sending a new version. Got it, thanks. > > > . >