Bug 1655792

Created attachment 1511139 [details]
coverity log

 In the Coverity logs, I see 4 occurrences of RESOURCE_LEAK.

 The first and second are in the function "virt_viewer_app_open_tunnel()".
 File descriptors are created by a call to "socketpair()", and after a call to "fork()", the parent keeps one, and the child keeps the other.
 At this point, the child closes its stdin and stdout descriptors, and uses "dup()" to copy its socket and use it as stdin and stdout. Coverity thinks that we're not keeping track of the file descriptor returned by dup(), but since stdin and stdout have just been closed, those duplicated descriptors are replacing them.
 This is a false positive, there is no leak here.


 The third happens in the function "virt_viewer_app_channel_open()".
 At the end of the function, the file descriptor that was previously opened is given as parameter to virt_viewer_session_channel_open_fd(), and Coverity reports that the file descriptor is not used or copied there, and is then lost at the end of the function.
 The call to virt_viewer_session_channel_open_fd() actually ends up calling the function pointer "_VirtViewerSessionClass->channel_open_fd()".
 This pointer is set either to virt_viewer_session_spice_channel_open_fd() or to virt_viewer_session_vnc_channel_open_fd(), depending on the protocol being used.
 - In the case of SPICE, the file descriptor is copied to an internal structure used by the channel later on. If the function succeeds, the file descriptor is not lost, and the caller can ignore this file descriptor. But if the function fails, the file descriptor may stay opened. We are not checking the error code from the call to virt_viewer_session_channel_open_fd(), so we may not be closing the file descriptor in case of an error.
 - In the case of VNC, there is nothing done - a warning actually says "channel_open_fd is not supported by VNC". In this case, the file descriptor is not closed or copied. On return, the file descriptor is then effectively lost.


 The fourth happens in the function virt_viewer_app_default_activate(). It is similar to the third: a socket is opened, and then provided to virt_viewer_session_open_fd(). This is also a call to a pointer function that depends on the protocol:
 - the VNC version is calling a gtk-vnc function with the file descriptor. I didn't verify yet what this function does to the file descriptor in case of failure, but since we are opening it, we're probably supposed to close it.
 - for the SPICE version, the underlying functions being called are the same as in the third item above, with the same result.


 I will add a check for the error code from these functions to make sure we close the file descriptor on errors.

Provided a fix upstream: https://gitlab.com/virt-viewer/virt-viewer/-/commit/d84149a92c8b7bd4b3cf82adb123426c50850702

Created attachment 1696778 [details]
virt-viewer-7.0

Created attachment 1696780 [details]
virt-viewer-9.0

Created attachment 1697792 [details]
coverity log for virt-viewer-7.0-9.el8

Created attachment 1697793 [details]
coverity log for virt-viewer-9.0-2.el8

Hi @Julien Ropé,
Got it, thanks for your detailed explanation, I move this bug from ON_QA to VERIFIED, thanks.

BR,
juzhou.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virt-viewer bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:4788