Re: [PATCH remote-viewer] Fix potential file descriptor leaks found by Coverity.

[Daniel P . Berrangé]

On Wed, Apr 01, 2020 at 09:25:18AM +0200, Julien Ropé wrote:
> The error code returned by virt_viewer_session_open_fd() and 
> virt_viewer_session_channel_open_fd() were not checked. The file descriptor 
> passed to them could then be left opened even if the function failed, causing 
> a leak of resources.
> 
> This was reported by a Coverity scan, logged under 
> https://bugzilla.redhat.com/show_bug.cgi?id=1655792
> 
> Signed-off-by: Julien Ropé 
> ---
>  src/virt-viewer-app.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH remote-viewer] Fix potential file descriptor leaks found by Coverity.

[Julien Ropé]

The error code returned by virt_viewer_session_open_fd() and 
virt_viewer_session_channel_open_fd() were not checked. The file descriptor 
passed to them could then be left opened even if the function failed, causing a 
leak of resources.

This was reported by a Coverity scan, logged under 
https://bugzilla.redhat.com/show_bug.cgi?id=1655792

Signed-off-by: Julien Ropé 
---
 src/virt-viewer-app.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
index a292ad8..12ee09d 100644
--- a/src/virt-viewer-app.c
+++ b/src/virt-viewer-app.c
@@ -1296,7 +1296,11 @@ virt_viewer_app_channel_open(VirtViewerSession *session,
 return;
 }
 
-virt_viewer_session_channel_open_fd(session, channel, fd);
+if (!virt_viewer_session_channel_open_fd(session, channel, fd)) {
+// in case of error, close the file descriptor to prevent a leak
+// NOTE: as VNC doesn't support channel_open, this function will 
always return false for this protocol.
+close(fd);
+}
 }
 #else
 static void
@@ -1355,7 +1359,10 @@ virt_viewer_app_default_activate(VirtViewerApp *self, 
GError **error)
 #endif
 
 if (fd >= 0) {
-return virt_viewer_session_open_fd(VIRT_VIEWER_SESSION(priv->session), 
fd);
+gboolean ret = 
virt_viewer_session_open_fd(VIRT_VIEWER_SESSION(priv->session), fd);
+if (!ret)
+close (fd);
+return ret ;
 } else if (priv->guri) {
 virt_viewer_app_trace(self, "Opening connection to display at %s", 
priv->guri);
 return 
virt_viewer_session_open_uri(VIRT_VIEWER_SESSION(priv->session), priv->guri, 
error);
-- 
2.24.1