qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/2] virtfs-proxy-helper: Fix han


From: Markus Armbruster
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/2] virtfs-proxy-helper: Fix handle leak to make Coverity happy
Date: Fri, 28 Nov 2014 08:29:22 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Gonglei <address@hidden> writes:

> On 2014/11/27 20:47, Markus Armbruster wrote:
>
>> <address@hidden> writes:
>> 
>>> From: Gonglei <address@hidden>
>>>
>>> Coverity report:
>>> (94) Event open_fn:  Returning handle opened by function "proxy_socket(char 
>>> const *, uid_t, gid_t)". [details]
>>> (95) Event var_assign:  Assigning: "sock" = handle returned from 
>>> "proxy_socket(sock_name, own_u, own_g)".
>>> (103) Event leaked_handle:  Handle variable "sock" going out of scope leaks 
>>> the handle.
>>>
>>> Signed-off-by: Gonglei <address@hidden>
>>> ---
>>>  fsdev/virtfs-proxy-helper.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
>>> index c1da2d7..2d72def 100644
>>> --- a/fsdev/virtfs-proxy-helper.c
>>> +++ b/fsdev/virtfs-proxy-helper.c
>>> @@ -1150,6 +1150,9 @@ int main(int argc, char **argv)
>>>  
>>>      process_requests(sock);
>>>  error:
>>> +    if (sock_name && sock >= 0) {
>>> +        close(sock);
>>> +    }
>>>      do_log(LOG_INFO, "Done\n");
>>>      closelog();
>>>      return 0;
>> 
>> Why if sock_name?  What about sock gotten from -f?
>>
>
> Thanks for your review, Makus :)
>
>  
>
> Because only sock_name is non-NULL, the sock returned from
> "proxy_socket(sock_name, own_u, own_g)", then will leak fd.
> If sock gotten from -f, maybe the caller will free it IMO.

Since this is main(), the "caller" is the OS.  And yes, the OS closes
the -f file descriptor when main() returns, because it closes *all* file
descriptors.  Calling close() before return from main() is pointless,
unless you check for errors.

Unfortunately, Coverity doesn't understand this.  Calling close() in
main() suppresses its bogus defect report.

>> If sock >= 0 is pointless, too, but needed to hush up Coverity.
>
> You mean do not check sock_name is NULL or not?

Yes, unless it causes another bogus Coverity defect.

Or simply mark the defect as invalid and move on without patching the
code.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]