qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] vnc: fix fd leak at vnc_display_open()
@ 2014-10-23  6:33 arei.gonglei
  2014-10-23  7:52 ` Gerd Hoffmann
  2014-10-23  7:57 ` Gerd Hoffmann
  0 siblings, 2 replies; 4+ messages in thread
From: arei.gonglei @ 2014-10-23  6:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gonglei, weidong.huang, kraxel, peter.huangpeng

From: Gonglei <arei.gonglei@huawei.com>

When using qmp change vnc interface, will leak fd
of vs->lsock and vs->lwebsock (if configed). Close
them before: re-evaluate.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
This patch based on my just prvious vnc patch seires.

Easy to reproduce this leak:
 $ ./qemu-system-x86_64 [...] -monitor stdio -vnc 0.0.0.0:10,password
(qemu) change vnc 0.0.0.0:11,password,sasl
(qemu) 
Then using vnc client connet the vnc server with 'port 10', Qemu will hang.
---
 ui/vnc.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 8f2f277..1f25b51 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3257,9 +3257,18 @@ void vnc_display_open(const char *display, Error **errp)
     if (reverse) {
         /* connect to viewer */
         int csock;
-        vs->lsock = -1;
+
+        if (vs->lsock != -1) {
+            qemu_set_fd_handler2(vs->lsock, NULL, NULL, NULL, NULL);
+            close(vs->lsock);
+            vs->lsock = -1;
+        }
 #ifdef CONFIG_VNC_WS
-        vs->lwebsock = -1;
+        if (vs->lwebsock != -1) {
+            qemu_set_fd_handler2(vs->lwebsock, NULL, NULL, NULL, NULL);
+            close(vs->lwebsock);
+            vs->lwebsock = -1;
+        }
 #endif
         if (strncmp(display, "unix:", 5) == 0) {
             csock = unix_connect(display+5, errp);
@@ -3274,6 +3283,11 @@ void vnc_display_open(const char *display, Error **errp)
         /* listen for connects */
         char *dpy;
         dpy = g_malloc(256);
+        if (vs->lsock != -1) {
+            qemu_set_fd_handler2(vs->lsock, NULL, NULL, NULL, NULL);
+            close(vs->lsock);
+            vs->lsock = -1;
+        }
         if (strncmp(display, "unix:", 5) == 0) {
             pstrcpy(dpy, 256, "unix:");
             vs->lsock = unix_listen(display+5, dpy+5, 256-5, errp);
@@ -3286,6 +3300,11 @@ void vnc_display_open(const char *display, Error **errp)
             }
 #ifdef CONFIG_VNC_WS
             if (vs->websocket) {
+                if (vs->lwebsock != -1) {
+                    qemu_set_fd_handler2(vs->lwebsock, NULL, NULL, NULL, NULL);
+                    close(vs->lwebsock);
+                    vs->lwebsock = -1;
+                }
                 if (vs->ws_display) {
                     vs->lwebsock = inet_listen(vs->ws_display, NULL, 256,
                         SOCK_STREAM, 0, errp);
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] vnc: fix fd leak at vnc_display_open()
  2014-10-23  6:33 [Qemu-devel] [PATCH] vnc: fix fd leak at vnc_display_open() arei.gonglei
@ 2014-10-23  7:52 ` Gerd Hoffmann
  2014-10-23  7:57 ` Gerd Hoffmann
  1 sibling, 0 replies; 4+ messages in thread
From: Gerd Hoffmann @ 2014-10-23  7:52 UTC (permalink / raw)
  To: arei.gonglei; +Cc: weidong.huang, qemu-devel, peter.huangpeng

On Do, 2014-10-23 at 14:33 +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> When using qmp change vnc interface, will leak fd
> of vs->lsock and vs->lwebsock (if configed). Close
> them before: re-evaluate.

I think we can do that before "if (reverse)", then we'll need the code
only once.

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] vnc: fix fd leak at vnc_display_open()
  2014-10-23  6:33 [Qemu-devel] [PATCH] vnc: fix fd leak at vnc_display_open() arei.gonglei
  2014-10-23  7:52 ` Gerd Hoffmann
@ 2014-10-23  7:57 ` Gerd Hoffmann
  2014-10-23  8:05   ` Gonglei
  1 sibling, 1 reply; 4+ messages in thread
From: Gerd Hoffmann @ 2014-10-23  7:57 UTC (permalink / raw)
  To: arei.gonglei; +Cc: weidong.huang, qemu-devel, peter.huangpeng

On Do, 2014-10-23 at 14:33 +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> When using qmp change vnc interface, will leak fd
> of vs->lsock and vs->lwebsock (if configed). Close
> them before: re-evaluate.

Ah, the file handles are leaked because you dropped the
vnc_display_close() call.  Don't do that then ...

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] vnc: fix fd leak at vnc_display_open()
  2014-10-23  7:57 ` Gerd Hoffmann
@ 2014-10-23  8:05   ` Gonglei
  0 siblings, 0 replies; 4+ messages in thread
From: Gonglei @ 2014-10-23  8:05 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: Huangweidong (C), qemu-devel@nongnu.org, Huangpeng (Peter)

On 2014/10/23 15:57, Gerd Hoffmann wrote:

> On Do, 2014-10-23 at 14:33 +0800, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> When using qmp change vnc interface, will leak fd
>> of vs->lsock and vs->lwebsock (if configed). Close
>> them before: re-evaluate.
> 
> Ah, the file handles are leaked because you dropped the
> vnc_display_close() call.  Don't do that then ...
> 
Oops, understand. Sorry for the noise :(


What's your suggestion about the problem described in cover-letter? Gerd


Best regards,
-Gonglei

 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-10-23  8:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-23  6:33 [Qemu-devel] [PATCH] vnc: fix fd leak at vnc_display_open() arei.gonglei
2014-10-23  7:52 ` Gerd Hoffmann
2014-10-23  7:57 ` Gerd Hoffmann
2014-10-23  8:05   ` Gonglei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).