From: Marc-André Lureau <marcandre.lureau@redhat.com>

Whether it is SPICE, VNC, D-Bus, or the socket chardev, they all
actually expect a socket kind or will fail in different ways at runtime.

Throw an error early if the given 'add_client' fd is not a socket, and
close it to avoid leaks.

This allows to replace the close() call with a more correct & portable
closesocket() version.

(this will allow importing sockets on Windows with a specialized command
in the following patch, while keeping the remaining monitor associated
sockets/add_client code & usage untouched)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 monitor/qmp-cmds.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 859012aef4..2dae6bb10f 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -14,6 +14,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/sockets.h"
 #include "monitor-internal.h"
 #include "monitor/qdev.h"
 #include "monitor/qmp-helpers.h"
@@ -139,11 +140,17 @@ void qmp_add_client(const char *protocol, const char *fdname,
         return;
     }
 
+    if (!fd_is_socket(fd)) {
+        error_setg(errp, "add_client expects a socket");
+        close(fd);
+        return;
+    }
+
     for (i = 0; i < ARRAY_SIZE(protocol_table); i++) {
         if (!strcmp(protocol, protocol_table[i].name)) {
             if (!protocol_table[i].add_client(fd, has_skipauth, skipauth,
                                               has_tls, tls, errp)) {
-                close(fd);
+                closesocket(fd);
             }
             return;
         }
@@ -151,7 +158,7 @@ void qmp_add_client(const char *protocol, const char *fdname,
 
     if (!qmp_add_client_char(fd, has_skipauth, skipauth, has_tls, tls,
                              protocol, errp)) {
-        close(fd);
+        closesocket(fd);
     }
 }
 
-- 
2.39.1

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Whether it is SPICE, VNC, D-Bus, or the socket chardev, they all
> actually expect a socket kind or will fail in different ways at runtime.
>
> Throw an error early if the given 'add_client' fd is not a socket, and
> close it to avoid leaks.
>
> This allows to replace the close() call with a more correct & portable
> closesocket() version.
>
> (this will allow importing sockets on Windows with a specialized command
> in the following patch, while keeping the remaining monitor associated
> sockets/add_client code & usage untouched)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  monitor/qmp-cmds.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 859012aef4..2dae6bb10f 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -14,6 +14,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/sockets.h"
>  #include "monitor-internal.h"
>  #include "monitor/qdev.h"
>  #include "monitor/qmp-helpers.h"
> @@ -139,11 +140,17 @@ void qmp_add_client(const char *protocol, const char *fdname,
>          return;
>      }
>  
> +    if (!fd_is_socket(fd)) {
> +        error_setg(errp, "add_client expects a socket");
> +        close(fd);
> +        return;
> +    }
> +
>      for (i = 0; i < ARRAY_SIZE(protocol_table); i++) {
>          if (!strcmp(protocol, protocol_table[i].name)) {
>              if (!protocol_table[i].add_client(fd, has_skipauth, skipauth,
>                                                has_tls, tls, errp)) {
> -                close(fd);
> +                closesocket(fd);
>              }
>              return;
>          }
> @@ -151,7 +158,7 @@ void qmp_add_client(const char *protocol, const char *fdname,
>  
>      if (!qmp_add_client_char(fd, has_skipauth, skipauth, has_tls, tls,
>                               protocol, errp)) {
> -        close(fd);
> +        closesocket(fd);
>      }
>  }

Please update add_client's doc comment in qapi/misc.json to state
explicitly that a socket is required.