diff mbox

[dpdk-dev] member: fix memory leak on error

Message ID D2C4A16CA39F7F4E8E384D204491D7A6445B26CC@ORSMSX105.amr.corp.intel.com (mailing list archive)
State Not Applicable, archived
Delegated to: Thomas Monjalon
Headers show

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply patch file failure

Commit Message

Wang, Yipeng1 Dec. 22, 2017, 6:33 p.m. UTC
>-----Original Message-----

>From: Burakov, Anatoly

>Yep, i can see that now. Didn't think to look inside rte_member_free()

>:/ However, you're creating a race condition there - you're unlocking a

>tailq, and then locking (and unlocking) it again inside

>rte_member_free() - it probably needs _thread_unsafe() functions that

>you can call from behind the lock.

>

>--


Thank you Anatoly,

I realize that rte_member_free does not do anything good here. As a fix, I think the following should work. Is there any other concern?

Thank you!

Comments

Burakov, Anatoly Dec. 23, 2017, 11:55 a.m. UTC | #1
On 22-Dec-17 6:33 PM, Wang, Yipeng1 wrote:
>> -----Original Message-----
>> From: Burakov, Anatoly
>> Yep, i can see that now. Didn't think to look inside rte_member_free()
>> :/ However, you're creating a race condition there - you're unlocking a
>> tailq, and then locking (and unlocking) it again inside
>> rte_member_free() - it probably needs _thread_unsafe() functions that
>> you can call from behind the lock.
>>
>> --
> 
> Thank you Anatoly,
> 
> I realize that rte_member_free does not do anything good here. As a fix, I think the following should work. Is there any other concern?
> 

Yes, that should work. Table creation is the last step that can cause an 
error, so if we're there, we already know that we couldn't have 
allocated it, so there's no need to deallocate those, and simple 
rte_free(setsum) should do. I'll submit a v2?
Wang, Yipeng1 Dec. 26, 2017, 5:23 p.m. UTC | #2
Sure. Thanks Anatoly,

Please go ahead and submit V2. 

Thanks
Yipeng

>-----Original Message-----

>From: Burakov, Anatoly

>Sent: Saturday, December 23, 2017 3:55 AM

>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; dev@dpdk.org

>Cc: Gobriel, Sameh <sameh.gobriel@intel.com>

>Subject: Re: [PATCH] member: fix memory leak on error

>

>On 22-Dec-17 6:33 PM, Wang, Yipeng1 wrote:

>>> -----Original Message-----

>>> From: Burakov, Anatoly

>>> Yep, i can see that now. Didn't think to look inside rte_member_free()

>>> :/ However, you're creating a race condition there - you're unlocking a

>>> tailq, and then locking (and unlocking) it again inside

>>> rte_member_free() - it probably needs _thread_unsafe() functions that

>>> you can call from behind the lock.

>>>

>>> --

>>

>> Thank you Anatoly,

>>

>> I realize that rte_member_free does not do anything good here. As a fix, I

>think the following should work. Is there any other concern?

>>

>

>Yes, that should work. Table creation is the last step that can cause an

>error, so if we're there, we already know that we couldn't have

>allocated it, so there's no need to deallocate those, and simple

>rte_free(setsum) should do. I'll submit a v2?

>

>--

>Thanks,

>Anatoly
diff mbox

Patch

diff --git a/lib/librte_member/rte_member.c b/lib/librte_member/rte_member.c
index cc9ea84..25934e8 100644
--- a/lib/librte_member/rte_member.c
+++ b/lib/librte_member/rte_member.c
@@ -192,7 +192,8 @@  rte_member_create(const struct rte_member_parameters *params)

 error_unlock_exit:
        rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
-       rte_member_free(setsum);
+       rte_free(te);
+       rte_free(setsum);
        return NULL;
 }