[PATCH v2 00/10] Coverity fixes for vchan-socket-proxy

[PATCH v2 00/10] Coverity fixes for vchan-socket-proxy

[Jason Andryuk]

This series addresses some Coverity reports.  To handle closing FDs, a
state struct is introduced to track FDs closed in both main() and
data_loop().

v2 changes "Ensure UNIX path NUL terminated" to avoid a warning with
gcc-10.  Also, "Move perror() into listen_socket" and "Move perror()
into connect_socket" are new.

Jason Andryuk (10):
  vchan-socket-proxy: Ensure UNIX path NUL terminated
  vchan-socket-proxy: Move perror() into listen_socket
  vchan-socket-proxy: Move perror() into connect_socket
  vchan-socket-proxy: Check xs_watch return value
  vchan-socket-proxy: Unify main return value
  vchan-socket-proxy: Use a struct to store state
  vchan-socket-proxy: Switch data_loop() to take state
  vchan-socket-proxy: Set closed FDs to -1
  vchan-socket-proxy: Cleanup resources on exit
  vchan-socket-proxy: Handle closing shared input/output_fd

 tools/libvchan/vchan-socket-proxy.c | 183 ++++++++++++++++++----------
 1 file changed, 119 insertions(+), 64 deletions(-)

-- 
2.25.1

[PATCH v2 01/10] vchan-socket-proxy: Ensure UNIX path NUL terminated

[Jason Andryuk]

Check the socket path length to ensure sun_path is NUL terminated.

This was spotted by Citrix's Coverity.

Also use strcpy to avoid a warning "'__builtin_strncpy' specified bound
108 equals destination size [-Werror=stringop-truncation]" flagged by
gcc 10.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
CC: Olaf Hering <olaf@aepfle.de>

With Ubuntu's gcc-10, which is a pre-release "gcc-10 (Ubuntu
10-20200411-0ubuntu1) 10.0.1 20200411 (experimental)", I couldn't
actualy generate the strncpy warning.

 tools/libvchan/vchan-socket-proxy.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 13700c5d67..6ae1d84143 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -148,12 +148,18 @@ static int connect_socket(const char *path_or_fd) {
         return fd;
     }
 
+    if (strlen(path_or_fd) >= sizeof(addr.sun_path)) {
+        fprintf(stderr, "UNIX socket path \"%s\" too long (%zd >= %zd)\n",
+                path_or_fd, strlen(path_or_fd), sizeof(addr.sun_path));
+        return -1;
+    }
+
     fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (fd == -1)
         return -1;
 
     addr.sun_family = AF_UNIX;
-    strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
+    strcpy(addr.sun_path, path_or_fd);
     if (connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
         close(fd);
         return -1;
@@ -174,13 +180,19 @@ static int listen_socket(const char *path_or_fd) {
         return fd;
     }
 
+    if (strlen(path_or_fd) >= sizeof(addr.sun_path)) {
+        fprintf(stderr, "UNIX socket path \"%s\" too long (%zd >= %zd)\n",
+                path_or_fd, strlen(path_or_fd), sizeof(addr.sun_path));
+        return -1;
+    }
+
     /* if not a number, assume a socket path */
     fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (fd == -1)
         return -1;
 
     addr.sun_family = AF_UNIX;
-    strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
+    strcpy(addr.sun_path, path_or_fd);
     if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
         close(fd);
         return -1;
-- 
2.25.1

[PATCH v2 02/10] vchan-socket-proxy: Move perror() into listen_socket

[Jason Andryuk]

The use of perror on the return from listen_socket can produce
misleading results like:
UNIX socket path "/tmp/aa....aa" too long (156 >= 108)
listen socket: Success

errno is reset by subsequent system & library calls, so it may be
inaccurate by the time listen_socket returns.  Call perror immediately
after failing system calls to print the proper message.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 6ae1d84143..4edc3a44f5 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -188,16 +188,20 @@ static int listen_socket(const char *path_or_fd) {
 
     /* if not a number, assume a socket path */
     fd = socket(AF_UNIX, SOCK_STREAM, 0);
-    if (fd == -1)
+    if (fd == -1) {
+        perror("socket");
         return -1;
+    }
 
     addr.sun_family = AF_UNIX;
     strcpy(addr.sun_path, path_or_fd);
     if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        perror("bind");
         close(fd);
         return -1;
     }
     if (listen(fd, 5) != 0) {
+        perror("listen");
         close(fd);
         return -1;
     }
@@ -419,7 +423,7 @@ int main(int argc, char **argv)
         } else {
             socket_fd = listen_socket(socket_path);
             if (socket_fd == -1) {
-                perror("listen socket");
+                fprintf(stderr, "listen socket failed\n");
                 return 1;
             }
         }
-- 
2.25.1

[PATCH v2 03/10] vchan-socket-proxy: Move perror() into connect_socket

[Jason Andryuk]

errno is reset by subsequent system & library calls, so it may be
inaccurate by the time connect_socket returns.  Call perror immediately
after failing system calls to print the proper message.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 4edc3a44f5..c6a83e4712 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -155,12 +155,15 @@ static int connect_socket(const char *path_or_fd) {
     }
 
     fd = socket(AF_UNIX, SOCK_STREAM, 0);
-    if (fd == -1)
+    if (fd == -1) {
+        perror("socket");
         return -1;
+    }
 
     addr.sun_family = AF_UNIX;
     strcpy(addr.sun_path, path_or_fd);
     if (connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        perror("connect");
         close(fd);
         return -1;
     }
@@ -457,7 +460,7 @@ int main(int argc, char **argv)
                 input_fd = output_fd = connect_socket(socket_path);
             }
             if (input_fd == -1) {
-                perror("connect socket");
+                fprintf(stderr, "connect_socket failed\n");
                 return 1;
             }
             if (data_loop(ctrl, input_fd, output_fd) != 0)
-- 
2.25.1

[PATCH v2 04/10] vchan-socket-proxy: Check xs_watch return value

[Jason Andryuk]

Check the return value of xs_watch and error out on failure.

This was found by Citrix's Coverity.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index c6a83e4712..196f6016b9 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -232,8 +232,15 @@ static struct libxenvchan *connect_vchan(int domid, const char *path) {
         goto out;
     }
     /* wait for vchan server to create *path* */
-    xs_watch(xs, path, "path");
-    xs_watch(xs, "@releaseDomain", "release");
+    if (!xs_watch(xs, path, "path")) {
+        fprintf(stderr, "xs_watch(%s) failed.\n", path);
+        goto out;
+    }
+    if (!xs_watch(xs, "@releaseDomain", "release")) {
+        fprintf(stderr, "xs_watch(@releaseDomain failed.\n");
+        goto out;
+    }
+
     while ((watch_ret = xs_read_watch(xs, &watch_num))) {
         /* don't care about exact which fired the watch */
         free(watch_ret);
-- 
2.25.1

[PATCH v2 05/10] vchan-socket-proxy: Unify main return value

[Jason Andryuk]

Introduce 'ret' for main's return value and remove direct returns.  This
is in preparation for a unified exit path with resource cleanup.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 196f6016b9..36a2fe2cb8 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -388,6 +388,7 @@ int main(int argc, char **argv)
     const char *vchan_path;
     const char *state_path = NULL;
     int opt;
+    int ret;
 
     while ((opt = getopt_long(argc, argv, "m:vs:", options, NULL)) != -1) {
         switch (opt) {
@@ -454,6 +455,8 @@ int main(int argc, char **argv)
         xs_close(xs);
     }
 
+    ret = 0;
+
     for (;;) {
         if (is_server) {
             /* wait for vchan connection */
@@ -468,7 +471,8 @@ int main(int argc, char **argv)
             }
             if (input_fd == -1) {
                 fprintf(stderr, "connect_socket failed\n");
-                return 1;
+                ret = 1;
+                break;
             }
             if (data_loop(ctrl, input_fd, output_fd) != 0)
                 break;
@@ -481,14 +485,16 @@ int main(int argc, char **argv)
                 input_fd = output_fd = accept(socket_fd, NULL, NULL);
             if (input_fd == -1) {
                 perror("accept");
-                return 1;
+                ret = 1;
+                break;
             }
             set_nonblocking(input_fd, 1);
             set_nonblocking(output_fd, 1);
             ctrl = connect_vchan(domid, vchan_path);
             if (!ctrl) {
                 perror("vchan client init");
-                return 1;
+                ret = 1;
+                break;
             }
             if (data_loop(ctrl, input_fd, output_fd) != 0)
                 break;
@@ -500,5 +506,6 @@ int main(int argc, char **argv)
             ctrl = NULL;
         }
     }
-    return 0;
+
+    return ret;
 }
-- 
2.25.1

[PATCH v2 06/10] vchan-socket-proxy: Use a struct to store state

[Jason Andryuk]

Use a struct to group the vchan ctrl and FDs.  This will facilite
tracking the state of open and closed FDs and ctrl in data_loop().

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 52 +++++++++++++++++------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 36a2fe2cb8..a932c94c97 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -89,6 +89,12 @@ int insiz = 0;
 int outsiz = 0;
 int verbose = 0;
 
+struct vchan_proxy_state {
+    struct libxenvchan *ctrl;
+    int output_fd;
+    int input_fd;
+};
+
 static void vchan_wr(struct libxenvchan *ctrl) {
     int ret;
 
@@ -381,8 +387,9 @@ int main(int argc, char **argv)
 {
     int is_server = 0;
     int socket_fd = -1;
-    int input_fd, output_fd;
-    struct libxenvchan *ctrl = NULL;
+    struct vchan_proxy_state state = { .ctrl = NULL,
+                                       .input_fd = -1,
+                                       .output_fd = -1 };
     const char *socket_path;
     int domid;
     const char *vchan_path;
@@ -422,15 +429,15 @@ int main(int argc, char **argv)
     socket_path = argv[optind+2];
 
     if (is_server) {
-        ctrl = libxenvchan_server_init(NULL, domid, vchan_path, 0, 0);
-        if (!ctrl) {
+        state.ctrl = libxenvchan_server_init(NULL, domid, vchan_path, 0, 0);
+        if (!state.ctrl) {
             perror("libxenvchan_server_init");
             exit(1);
         }
     } else {
         if (strcmp(socket_path, "-") == 0) {
-            input_fd = 0;
-            output_fd = 1;
+            state.input_fd = 0;
+            state.output_fd = 1;
         } else {
             socket_fd = listen_socket(socket_path);
             if (socket_fd == -1) {
@@ -460,21 +467,21 @@ int main(int argc, char **argv)
     for (;;) {
         if (is_server) {
             /* wait for vchan connection */
-            while (libxenvchan_is_open(ctrl) != 1)
-                libxenvchan_wait(ctrl);
+            while (libxenvchan_is_open(state.ctrl) != 1)
+                libxenvchan_wait(state.ctrl);
             /* vchan client connected, setup local FD if needed */
             if (strcmp(socket_path, "-") == 0) {
-                input_fd = 0;
-                output_fd = 1;
+                state.input_fd = 0;
+                state.output_fd = 1;
             } else {
-                input_fd = output_fd = connect_socket(socket_path);
+                state.input_fd = state.output_fd = connect_socket(socket_path);
             }
-            if (input_fd == -1) {
+            if (state.input_fd == -1) {
                 fprintf(stderr, "connect_socket failed\n");
                 ret = 1;
                 break;
             }
-            if (data_loop(ctrl, input_fd, output_fd) != 0)
+            if (data_loop(state.ctrl, state.input_fd, state.output_fd) != 0)
                 break;
             /* keep it running only when get UNIX socket path */
             if (socket_path[0] != '/')
@@ -482,28 +489,29 @@ int main(int argc, char **argv)
         } else {
             /* wait for local socket connection */
             if (strcmp(socket_path, "-") != 0)
-                input_fd = output_fd = accept(socket_fd, NULL, NULL);
-            if (input_fd == -1) {
+                state.input_fd = state.output_fd = accept(socket_fd,
+                                                          NULL, NULL);
+            if (state.input_fd == -1) {
                 perror("accept");
                 ret = 1;
                 break;
             }
-            set_nonblocking(input_fd, 1);
-            set_nonblocking(output_fd, 1);
-            ctrl = connect_vchan(domid, vchan_path);
-            if (!ctrl) {
+            set_nonblocking(state.input_fd, 1);
+            set_nonblocking(state.output_fd, 1);
+            state.ctrl = connect_vchan(domid, vchan_path);
+            if (!state.ctrl) {
                 perror("vchan client init");
                 ret = 1;
                 break;
             }
-            if (data_loop(ctrl, input_fd, output_fd) != 0)
+            if (data_loop(state.ctrl, state.input_fd, state.output_fd) != 0)
                 break;
             /* don't reconnect if output was stdout */
             if (strcmp(socket_path, "-") == 0)
                 break;
 
-            libxenvchan_close(ctrl);
-            ctrl = NULL;
+            libxenvchan_close(state.ctrl);
+            state.ctrl = NULL;
         }
     }
 
-- 
2.25.1

[PATCH v2 07/10] vchan-socket-proxy: Switch data_loop() to take state

[Jason Andryuk]

Switch data_loop to take a pointer to vchan_proxy_state.

No functional change.

This removes a dead store to input_fd identified by Coverity.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 65 +++++++++++++++--------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index a932c94c97..29a12260ed 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -286,13 +286,13 @@ static void discard_buffers(struct libxenvchan *ctrl) {
     }
 }
 
-int data_loop(struct libxenvchan *ctrl, int input_fd, int output_fd)
+int data_loop(struct vchan_proxy_state *state)
 {
     int ret;
     int libxenvchan_fd;
     int max_fd;
 
-    libxenvchan_fd = libxenvchan_fd_for_select(ctrl);
+    libxenvchan_fd = libxenvchan_fd_for_select(state->ctrl);
     for (;;) {
         fd_set rfds;
         fd_set wfds;
@@ -300,15 +300,15 @@ int data_loop(struct libxenvchan *ctrl, int input_fd, int output_fd)
         FD_ZERO(&wfds);
 
         max_fd = -1;
-        if (input_fd != -1 && insiz != BUFSIZE) {
-            FD_SET(input_fd, &rfds);
-            if (input_fd > max_fd)
-                max_fd = input_fd;
+        if (state->input_fd != -1 && insiz != BUFSIZE) {
+            FD_SET(state->input_fd, &rfds);
+            if (state->input_fd > max_fd)
+                max_fd = state->input_fd;
         }
-        if (output_fd != -1 && outsiz) {
-            FD_SET(output_fd, &wfds);
-            if (output_fd > max_fd)
-                max_fd = output_fd;
+        if (state->output_fd != -1 && outsiz) {
+            FD_SET(state->output_fd, &wfds);
+            if (state->output_fd > max_fd)
+                max_fd = state->output_fd;
         }
         FD_SET(libxenvchan_fd, &rfds);
         if (libxenvchan_fd > max_fd)
@@ -319,52 +319,53 @@ int data_loop(struct libxenvchan *ctrl, int input_fd, int output_fd)
             exit(1);
         }
         if (FD_ISSET(libxenvchan_fd, &rfds)) {
-            libxenvchan_wait(ctrl);
-            if (!libxenvchan_is_open(ctrl)) {
+            libxenvchan_wait(state->ctrl);
+            if (!libxenvchan_is_open(state->ctrl)) {
                 if (verbose)
                     fprintf(stderr, "vchan client disconnected\n");
                 while (outsiz)
-                    socket_wr(output_fd);
-                close(output_fd);
-                close(input_fd);
-                discard_buffers(ctrl);
+                    socket_wr(state->output_fd);
+                close(state->output_fd);
+                close(state->input_fd);
+                discard_buffers(state->ctrl);
                 break;
             }
-            vchan_wr(ctrl);
+            vchan_wr(state->ctrl);
         }
 
-        if (FD_ISSET(input_fd, &rfds)) {
-            ret = read(input_fd, inbuf + insiz, BUFSIZE - insiz);
+        if (FD_ISSET(state->input_fd, &rfds)) {
+            ret = read(state->input_fd, inbuf + insiz, BUFSIZE - insiz);
             if (ret < 0 && errno != EAGAIN)
                 exit(1);
             if (verbose)
                 fprintf(stderr, "from-unix: %.*s\n", ret, inbuf + insiz);
             if (ret == 0) {
                 /* EOF on socket, write everything in the buffer and close the
-                 * input_fd socket */
+                 * state->input_fd socket */
                 while (insiz) {
-                    vchan_wr(ctrl);
-                    libxenvchan_wait(ctrl);
+                    vchan_wr(state->ctrl);
+                    libxenvchan_wait(state->ctrl);
                 }
-                close(input_fd);
-                input_fd = -1;
+                close(state->input_fd);
+                state->input_fd = -1;
                 /* TODO: maybe signal the vchan client somehow? */
                 break;
             }
             if (ret)
                 insiz += ret;
-            vchan_wr(ctrl);
+            vchan_wr(state->ctrl);
         }
-        if (FD_ISSET(output_fd, &wfds))
-            socket_wr(output_fd);
-        while (libxenvchan_data_ready(ctrl) && outsiz < BUFSIZE) {
-            ret = libxenvchan_read(ctrl, outbuf + outsiz, BUFSIZE - outsiz);
+        if (FD_ISSET(state->output_fd, &wfds))
+            socket_wr(state->output_fd);
+        while (libxenvchan_data_ready(state->ctrl) && outsiz < BUFSIZE) {
+            ret = libxenvchan_read(state->ctrl, outbuf + outsiz,
+                                   BUFSIZE - outsiz);
             if (ret < 0)
                 exit(1);
             if (verbose)
                 fprintf(stderr, "from-vchan: %.*s\n", ret, outbuf + outsiz);
             outsiz += ret;
-            socket_wr(output_fd);
+            socket_wr(state->output_fd);
         }
     }
     return 0;
@@ -481,7 +482,7 @@ int main(int argc, char **argv)
                 ret = 1;
                 break;
             }
-            if (data_loop(state.ctrl, state.input_fd, state.output_fd) != 0)
+            if (data_loop(&state) != 0)
                 break;
             /* keep it running only when get UNIX socket path */
             if (socket_path[0] != '/')
@@ -504,7 +505,7 @@ int main(int argc, char **argv)
                 ret = 1;
                 break;
             }
-            if (data_loop(state.ctrl, state.input_fd, state.output_fd) != 0)
+            if (data_loop(&state) != 0)
                 break;
             /* don't reconnect if output was stdout */
             if (strcmp(socket_path, "-") == 0)
-- 
2.25.1

[PATCH v2 08/10] vchan-socket-proxy: Set closed FDs to -1

[Jason Andryuk]

These FDs are closed, so set them to -1 so they are no longer valid.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 29a12260ed..cd7629bc4e 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -326,7 +326,9 @@ int data_loop(struct vchan_proxy_state *state)
                 while (outsiz)
                     socket_wr(state->output_fd);
                 close(state->output_fd);
+                state->output_fd = -1;
                 close(state->input_fd);
+                state->input_fd = -1;
                 discard_buffers(state->ctrl);
                 break;
             }
-- 
2.25.1

[PATCH v2 09/10] vchan-socket-proxy: Cleanup resources on exit

[Jason Andryuk]

Close open FDs and close th vchan connection when exiting the program.

This addresses some Coverity findings about leaking file descriptors.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index cd7629bc4e..3552783ec2 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -518,5 +518,14 @@ int main(int argc, char **argv)
         }
     }
 
+    if (state.output_fd >= 0)
+        close(state.output_fd);
+    if (state.input_fd >= 0)
+        close(state.input_fd);
+    if (state.ctrl)
+        libxenvchan_close(state.ctrl);
+    if (socket_fd >= 0)
+        close(socket_fd);
+
     return ret;
 }
-- 
2.25.1

[PATCH v2 10/10] vchan-socket-proxy: Handle closing shared input/output_fd

[Jason Andryuk]

input_fd & output_fd may be the same FD.  In that case, mark both as -1
when closing one.  That avoids a dangling FD reference.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 3552783ec2..e1d959c6d1 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -349,6 +349,8 @@ int data_loop(struct vchan_proxy_state *state)
                     libxenvchan_wait(state->ctrl);
                 }
                 close(state->input_fd);
+                if (state->input_fd == state->output_fd)
+                    state->output_fd = -1;
                 state->input_fd = -1;
                 /* TODO: maybe signal the vchan client somehow? */
                 break;
-- 
2.25.1

Re: [PATCH v2 00/10] Coverity fixes for vchan-socket-proxy

[Wei Liu]

On Wed, Jun 10, 2020 at 11:29:26PM -0400, Jason Andryuk wrote:
> This series addresses some Coverity reports.  To handle closing FDs, a
> state struct is introduced to track FDs closed in both main() and
> data_loop().
> 
> v2 changes "Ensure UNIX path NUL terminated" to avoid a warning with
> gcc-10.  Also, "Move perror() into listen_socket" and "Move perror()
> into connect_socket" are new.
> 
> Jason Andryuk (10):
>   vchan-socket-proxy: Ensure UNIX path NUL terminated
>   vchan-socket-proxy: Move perror() into listen_socket
>   vchan-socket-proxy: Move perror() into connect_socket
>   vchan-socket-proxy: Check xs_watch return value
>   vchan-socket-proxy: Unify main return value
>   vchan-socket-proxy: Use a struct to store state
>   vchan-socket-proxy: Switch data_loop() to take state
>   vchan-socket-proxy: Set closed FDs to -1
>   vchan-socket-proxy: Cleanup resources on exit
>   vchan-socket-proxy: Handle closing shared input/output_fd

Acked-by: Wei Liu <wl@xen.org>

Cc Paul. V1 of this series was posted back in May. I consider this
series bug fixes, so they should be applied for 4.14. The risk is low
because vchan-socket-proxy is a small utility used by a small number of
users.

Marek, you gave Review tags in v1. Do they still apply here?

> 
>  tools/libvchan/vchan-socket-proxy.c | 183 ++++++++++++++++++----------
>  1 file changed, 119 insertions(+), 64 deletions(-)
> 
> -- 
> 2.25.1
> 

RE: [PATCH v2 00/10] Coverity fixes for vchan-socket-proxy

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu <wl@xen.org>
> Sent: 26 June 2020 11:18
> To: Jason Andryuk <jandryuk@gmail.com>
> Cc: xen-devel@lists.xenproject.org; Ian Jackson <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>;
> Paul Durrant <paul@xen.org>; marmarek@invisiblethingslab.com
> Subject: Re: [PATCH v2 00/10] Coverity fixes for vchan-socket-proxy
> 
> On Wed, Jun 10, 2020 at 11:29:26PM -0400, Jason Andryuk wrote:
> > This series addresses some Coverity reports.  To handle closing FDs, a
> > state struct is introduced to track FDs closed in both main() and
> > data_loop().
> >
> > v2 changes "Ensure UNIX path NUL terminated" to avoid a warning with
> > gcc-10.  Also, "Move perror() into listen_socket" and "Move perror()
> > into connect_socket" are new.
> >
> > Jason Andryuk (10):
> >   vchan-socket-proxy: Ensure UNIX path NUL terminated
> >   vchan-socket-proxy: Move perror() into listen_socket
> >   vchan-socket-proxy: Move perror() into connect_socket
> >   vchan-socket-proxy: Check xs_watch return value
> >   vchan-socket-proxy: Unify main return value
> >   vchan-socket-proxy: Use a struct to store state
> >   vchan-socket-proxy: Switch data_loop() to take state
> >   vchan-socket-proxy: Set closed FDs to -1
> >   vchan-socket-proxy: Cleanup resources on exit
> >   vchan-socket-proxy: Handle closing shared input/output_fd
> 
> Acked-by: Wei Liu <wl@xen.org>
> 
> Cc Paul. V1 of this series was posted back in May. I consider this
> series bug fixes, so they should be applied for 4.14. The risk is low
> because vchan-socket-proxy is a small utility used by a small number of
> users.
> 

Agreed. Series...

Release-acked-by: Paul Durrant <paul@xen.org>

> Marek, you gave Review tags in v1. Do they still apply here?
> 
> >
> >  tools/libvchan/vchan-socket-proxy.c | 183 ++++++++++++++++++----------
> >  1 file changed, 119 insertions(+), 64 deletions(-)
> >
> > --
> > 2.25.1
> >

Re: [PATCH v2 00/10] Coverity fixes for vchan-socket-proxy

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On Fri, Jun 26, 2020 at 10:18:07AM +0000, Wei Liu wrote:
> On Wed, Jun 10, 2020 at 11:29:26PM -0400, Jason Andryuk wrote:
> > This series addresses some Coverity reports.  To handle closing FDs, a
> > state struct is introduced to track FDs closed in both main() and
> > data_loop().
> > 
> > v2 changes "Ensure UNIX path NUL terminated" to avoid a warning with
> > gcc-10.  Also, "Move perror() into listen_socket" and "Move perror()
> > into connect_socket" are new.
> > 
> > Jason Andryuk (10):
> >   vchan-socket-proxy: Ensure UNIX path NUL terminated
> >   vchan-socket-proxy: Move perror() into listen_socket
> >   vchan-socket-proxy: Move perror() into connect_socket
> >   vchan-socket-proxy: Check xs_watch return value
> >   vchan-socket-proxy: Unify main return value
> >   vchan-socket-proxy: Use a struct to store state
> >   vchan-socket-proxy: Switch data_loop() to take state
> >   vchan-socket-proxy: Set closed FDs to -1
> >   vchan-socket-proxy: Cleanup resources on exit
> >   vchan-socket-proxy: Handle closing shared input/output_fd
> 
> Acked-by: Wei Liu <wl@xen.org>
> 
> Cc Paul. V1 of this series was posted back in May. I consider this
> series bug fixes, so they should be applied for 4.14. The risk is low
> because vchan-socket-proxy is a small utility used by a small number of
> users.
> 
> Marek, you gave Review tags in v1. Do they still apply here?

Yes. And also for the new patches 2-3. I thought I've posted it here,
but must have missed clicking "send"...

> > 
> >  tools/libvchan/vchan-socket-proxy.c | 183 ++++++++++++++++++----------
> >  1 file changed, 119 insertions(+), 64 deletions(-)
> > 
> > -- 
> > 2.25.1
> > 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 00/10] Coverity fixes for vchan-socket-proxy

[Wei Liu]

On Fri, Jun 26, 2020 at 01:12:01PM +0200, Marek Marczykowski-Górecki wrote:
> On Fri, Jun 26, 2020 at 10:18:07AM +0000, Wei Liu wrote:
> > On Wed, Jun 10, 2020 at 11:29:26PM -0400, Jason Andryuk wrote:
> > > This series addresses some Coverity reports.  To handle closing FDs, a
> > > state struct is introduced to track FDs closed in both main() and
> > > data_loop().
> > > 
> > > v2 changes "Ensure UNIX path NUL terminated" to avoid a warning with
> > > gcc-10.  Also, "Move perror() into listen_socket" and "Move perror()
> > > into connect_socket" are new.
> > > 
> > > Jason Andryuk (10):
> > >   vchan-socket-proxy: Ensure UNIX path NUL terminated
> > >   vchan-socket-proxy: Move perror() into listen_socket
> > >   vchan-socket-proxy: Move perror() into connect_socket
> > >   vchan-socket-proxy: Check xs_watch return value
> > >   vchan-socket-proxy: Unify main return value
> > >   vchan-socket-proxy: Use a struct to store state
> > >   vchan-socket-proxy: Switch data_loop() to take state
> > >   vchan-socket-proxy: Set closed FDs to -1
> > >   vchan-socket-proxy: Cleanup resources on exit
> > >   vchan-socket-proxy: Handle closing shared input/output_fd
> > 
> > Acked-by: Wei Liu <wl@xen.org>
> > 
> > Cc Paul. V1 of this series was posted back in May. I consider this
> > series bug fixes, so they should be applied for 4.14. The risk is low
> > because vchan-socket-proxy is a small utility used by a small number of
> > users.
> > 
> > Marek, you gave Review tags in v1. Do they still apply here?
> 
> Yes. And also for the new patches 2-3. I thought I've posted it here,
> but must have missed clicking "send"...

Thanks for confirming.

Wei.