[dpdk-dev] [PATCH v3 5/9] mem: fix potential resource leak

[Burakov, Anatoly]

On 27-Apr-18 5:27 PM, Bruce Richardson wrote:
> On Fri, Apr 27, 2018 at 04:55:51PM +0100, Burakov, Anatoly wrote:
>> On 27-Apr-18 4:21 PM, Bruce Richardson wrote:
>>> On Wed, Apr 25, 2018 at 10:56:43AM +0100, Anatoly Burakov wrote:
>>>> Normally, tailq entry should have a valid fd by the time we attempt
>>>> to map the segment. However, in case it doesn't, we're leaking fd,
>>>> so fix it.
>>>>
>>>> Coverity issue: 272570
>>>>
>>>> Fixes: 2a04139f66b4 ("eal: add single file segments option")
>>>> Cc: anatoly.burakov at intel.com
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
>>>> ---
>>>>    lib/librte_eal/linuxapp/eal/eal_memalloc.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
>>>> index fab5a98..b02e3a5 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
>>>> +++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
>>>> @@ -524,6 +524,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
>>>>    			if (te != NULL && te->fd >= 0) {
>>>>    				close(te->fd);
>>>>    				te->fd = -1;
>>>
>>> Is "fd" still not being leaked here, since we won't hit the else case and
>>> then jump to the end of the function where it goes out of scope?
>>
>> Perhaps i should clarify - te->fd and fd are the same fd.
>>
> Can you clarify that to coverity somehow?
> 

I don't think i can. The fd comes from get_seg_fd(), which finds the 
tailq entry and either returns existing fd, or opens a new one - and the 
same tailq entry is later looked up by alloc_seg(). Technically, of 
course, tailq contents might change inbetween the calls, but really 
that's not possible as only one thread in any given process is ever 
running through this code.

-- 
Thanks,
Anatoly