qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/1] slirp branch
@ 2025-10-05 19:47 Samuel Thibault
  2025-10-05 19:47 ` [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket Samuel Thibault
  2025-10-06 21:59 ` [PULL 0/1] slirp branch Richard Henderson
  0 siblings, 2 replies; 6+ messages in thread
From: Samuel Thibault @ 2025-10-05 19:47 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Samuel Thibault, stefanha, jan.kiszka

The following changes since commit 6d10e021318b16e3e90f98b7b2fa187826e26c0a:

  Add a feature for mapping a host unix socket to a guest tcp socket (2025-10-05 21:13:11 +0200)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 6d10e021318b16e3e90f98b7b2fa187826e26c0a:

  Add a feature for mapping a host unix socket to a guest tcp socket (2025-10-05 21:13:11 +0200)

----------------------------------------------------------------
Add a feature for mapping a host unix socket to a guest tcp socket

----------------------------------------------------------------


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

* [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket
  2025-10-05 19:47 [PULL 0/1] slirp branch Samuel Thibault
@ 2025-10-05 19:47 ` Samuel Thibault
  2025-10-23  9:58   ` Peter Maydell
  2025-10-06 21:59 ` [PULL 0/1] slirp branch Richard Henderson
  1 sibling, 1 reply; 6+ messages in thread
From: Samuel Thibault @ 2025-10-05 19:47 UTC (permalink / raw)
  To: qemu-devel, peter.maydell
  Cc: Viktor Kurilko, stefanha, jan.kiszka, Samuel Thibault

From: Viktor Kurilko <murlockkinght@gmail.com>

This patch adds the ability to map a host unix socket to a guest tcp socket when
using the slirp backend. This feature was added in libslirp version 4.7.0.

A new syntax for unix socket: -hostfwd=unix:hostpath-[guestaddr]:guestport

Signed-off-by: Viktor Kurilko <murlockkinght@gmail.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-ID: <20250808143904.363907-1-murlockkinght@gmail.com>
---
 docs/system/devices/net.rst |   2 +-
 hmp-commands.hx             |   4 +-
 net/slirp.c                 | 114 +++++++++++++++++++++++++++---------
 qapi/net.json               |   2 +-
 qemu-options.hx             |  11 +++-
 5 files changed, 100 insertions(+), 33 deletions(-)

diff --git a/docs/system/devices/net.rst b/docs/system/devices/net.rst
index 7d76fe88c4..13199a44fd 100644
--- a/docs/system/devices/net.rst
+++ b/docs/system/devices/net.rst
@@ -79,7 +79,7 @@ those sockets. To allow ping for GID 100 (usually users group)::
 
 When using the built-in TFTP server, the router is also the TFTP server.
 
-When using the ``'-netdev user,hostfwd=...'`` option, TCP or UDP
+When using the ``'-netdev user,hostfwd=...'`` option, TCP, UDP or UNIX
 connections can be redirected from the host to the guest. It allows for
 example to redirect X11, telnet or SSH connections.
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 3cace8f1f7..15f6082596 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1357,8 +1357,8 @@ ERST
     {
         .name       = "hostfwd_add",
         .args_type  = "arg1:s,arg2:s?",
-        .params     = "[netdev_id] [tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport",
-        .help       = "redirect TCP or UDP connections from host to guest (requires -net user)",
+        .params     = "[netdev_id] [tcp|udp|unix]:[[hostaddr]:hostport|hostpath]-[guestaddr]:guestport",
+        .help       = "redirect TCP, UDP or UNIX connections from host to guest (requires -net user)",
         .cmd        = hmp_hostfwd_add,
     },
 #endif
diff --git a/net/slirp.c b/net/slirp.c
index 0a1c2a5eac..c627a9dd24 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -732,6 +732,7 @@ static SlirpState *slirp_lookup(Monitor *mon, const char *id)
 
 void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 {
+    /* TODO: support removing unix fwd */
     struct sockaddr_in host_addr = {
         .sin_family = AF_INET,
         .sin_addr = {
@@ -800,12 +801,13 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 
 static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
 {
-    struct sockaddr_in host_addr = {
-        .sin_family = AF_INET,
-        .sin_addr = {
-            .s_addr = INADDR_ANY,
-        },
-    };
+    union {
+        struct sockaddr_in in;
+#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
+        struct sockaddr_un un;
+#endif
+    } host_addr = {0};
+
     struct sockaddr_in guest_addr = {
         .sin_family = AF_INET,
         .sin_addr = {
@@ -816,9 +818,13 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
     int host_port, guest_port;
     const char *p;
     char buf[256];
-    int is_udp;
+    int is_udp = 0;
+#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
+    int is_unix = 0;
+#endif
     const char *end;
     const char *fail_reason = "Unknown reason";
+    socklen_t host_addr_size;
 
     p = redir_str;
     if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
@@ -829,30 +835,83 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
         is_udp = 0;
     } else if (!strcmp(buf, "udp")) {
         is_udp = 1;
-    } else {
-        fail_reason = "Bad protocol name";
-        goto fail_syntax;
     }
-
-    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        fail_reason = "Missing : separator";
-        goto fail_syntax;
+#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
+    else if (!strcmp(buf, "unix")) {
+        is_unix = 1;
     }
-    if (buf[0] != '\0' && !inet_aton(buf, &host_addr.sin_addr)) {
-        fail_reason = "Bad host address";
+#endif
+    else {
+        fail_reason = "Bad protocol name";
         goto fail_syntax;
     }
 
-    if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
-        fail_reason = "Bad host port separator";
-        goto fail_syntax;
-    }
-    err = qemu_strtoi(buf, &end, 0, &host_port);
-    if (err || host_port < 0 || host_port > 65535) {
-        fail_reason = "Bad host port";
-        goto fail_syntax;
+#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
+    if (is_unix) {
+        if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
+            fail_reason = "Missing - separator";
+            goto fail_syntax;
+        }
+        if (buf[0] == '\0') {
+            fail_reason = "Missing unix socket path";
+            goto fail_syntax;
+        }
+        if (buf[0] != '/') {
+            fail_reason = "unix socket path must be absolute";
+            goto fail_syntax;
+        }
+
+        size_t path_len = strlen(buf);
+        if (path_len > sizeof(host_addr.un.sun_path) - 1) {
+            fail_reason = "Unix socket path is too long";
+            goto fail_syntax;
+        }
+
+        struct stat st;
+        if (stat(buf, &st) == 0) {
+            if (!S_ISSOCK(st.st_mode)) {
+                fail_reason = "file exists and it's not unix socket";
+                goto fail_syntax;
+            }
+
+            if (unlink(buf) < 0) {
+                error_setg_errno(errp, errno, "Failed to unlink '%s'", buf);
+                goto fail_syntax;
+            }
+        }
+        host_addr.un.sun_family = AF_UNIX;
+        memcpy(host_addr.un.sun_path, buf, path_len);
+        host_addr_size = sizeof(host_addr.un);
+    } else
+#endif
+    {
+        host_addr.in.sin_family = AF_INET;
+        host_addr.in.sin_addr.s_addr = INADDR_ANY;
+
+        if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+            fail_reason = "Missing : separator";
+            goto fail_syntax;
+        }
+
+        if (buf[0] != '\0' && !inet_aton(buf, &host_addr.in.sin_addr)) {
+            fail_reason = "Bad host address";
+            goto fail_syntax;
+        }
+
+        if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
+            fail_reason = "Bad host port separator";
+            goto fail_syntax;
+        }
+
+        err = qemu_strtoi(buf, &end, 0, &host_port);
+        if (err || host_port < 0 || host_port > 65535) {
+            fail_reason = "Bad host port";
+            goto fail_syntax;
+        }
+
+        host_addr.in.sin_port = htons(host_port);
+        host_addr_size = sizeof(host_addr.in);
     }
-    host_addr.sin_port = htons(host_port);
 
     if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
         fail_reason = "Missing guest address";
@@ -872,12 +931,13 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
 
 #if SLIRP_CHECK_VERSION(4, 5, 0)
     err = slirp_add_hostxfwd(s->slirp,
-            (struct sockaddr *) &host_addr, sizeof(host_addr),
+            (struct sockaddr *) &host_addr, host_addr_size,
             (struct sockaddr *) &guest_addr, sizeof(guest_addr),
             is_udp ? SLIRP_HOSTFWD_UDP : 0);
 #else
+    (void) host_addr_size;
     err = slirp_add_hostfwd(s->slirp, is_udp,
-            host_addr.sin_addr, host_port,
+            host_addr.in.sin_addr, host_port,
             guest_addr.sin_addr, guest_port);
 #endif
 
diff --git a/qapi/net.json b/qapi/net.json
index 78bcc9871e..60d196afe5 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -281,7 +281,7 @@
 #
 # @smbserver: IP address of the built-in SMB server
 #
-# @hostfwd: redirect incoming TCP or UDP host connections to guest
+# @hostfwd: redirect incoming TCP, UDP or UNIX host connections to guest
 #     endpoints
 #
 # @guestfwd: forward guest TCP connections
diff --git a/qemu-options.hx b/qemu-options.hx
index 075f4be2e3..a9d640d9e6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3324,8 +3324,8 @@ SRST
 
         Note that a SAMBA server must be installed on the host OS.
 
-    ``hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport``
-        Redirect incoming TCP or UDP connections to the host port
+    ``hostfwd=[tcp|udp|unix]:[[hostaddr]:hostport|hostpath]-[guestaddr]:guestport``
+        Redirect incoming TCP, UDP or UNIX connections to the host port
         hostport to the guest IP address guestaddr on guest port
         guestport. If guestaddr is not specified, its value is x.x.x.15
         (default first address given by the built-in DHCP server). By
@@ -3355,6 +3355,13 @@ SRST
         Then when you use on the host ``telnet localhost 5555``, you
         connect to the guest telnet server.
 
+        To redirect host unix socket /tmp/vm to guest tcp socket 23 use
+        following:
+
+        .. parsed-literal::
+            # on the host
+            |qemu_system| -nic user,hostfwd=unix:/tmp/vm-:23
+
     ``guestfwd=[tcp]:server:port-dev``; \ ``guestfwd=[tcp]:server:port-cmd:command``
         Forward guest TCP connections to the IP address server on port
         port to the character device dev or to a program executed by
-- 
2.51.0



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

* Re: [PULL 0/1] slirp branch
  2025-10-05 19:47 [PULL 0/1] slirp branch Samuel Thibault
  2025-10-05 19:47 ` [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket Samuel Thibault
@ 2025-10-06 21:59 ` Richard Henderson
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2025-10-06 21:59 UTC (permalink / raw)
  To: qemu-devel

On 10/5/25 12:47, Samuel Thibault wrote:
> The following changes since commit 6d10e021318b16e3e90f98b7b2fa187826e26c0a:
> 
>    Add a feature for mapping a host unix socket to a guest tcp socket (2025-10-05 21:13:11 +0200)
> 
> are available in the Git repository at:
> 
>    https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
> 
> for you to fetch changes up to 6d10e021318b16e3e90f98b7b2fa187826e26c0a:
> 
>    Add a feature for mapping a host unix socket to a guest tcp socket (2025-10-05 21:13:11 +0200)
> 
> ----------------------------------------------------------------
> Add a feature for mapping a host unix socket to a guest tcp socket
> 
> ----------------------------------------------------------------
> 


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/10.2 as appropriate.

r~


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

* Re: [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket
  2025-10-05 19:47 ` [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket Samuel Thibault
@ 2025-10-23  9:58   ` Peter Maydell
  2025-10-23 10:10     ` Samuel Thibault
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2025-10-23  9:58 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: qemu-devel, Viktor Kurilko, stefanha, jan.kiszka

On Sun, 5 Oct 2025 at 20:48, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
>
> From: Viktor Kurilko <murlockkinght@gmail.com>
>
> This patch adds the ability to map a host unix socket to a guest tcp socket when
> using the slirp backend. This feature was added in libslirp version 4.7.0.
>
> A new syntax for unix socket: -hostfwd=unix:hostpath-[guestaddr]:guestport
>
> Signed-off-by: Viktor Kurilko <murlockkinght@gmail.com>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Message-ID: <20250808143904.363907-1-murlockkinght@gmail.com>
> ---

Coverity worries here about a possible time-of-check-time-of-use
bug (CID 1641394). This is a heuristic that tends to fire even
when there's no interesting attack possible, but I don't
know what this code is doing so I raise it here:

> +#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
> +    if (is_unix) {
> +        if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
> +            fail_reason = "Missing - separator";
> +            goto fail_syntax;
> +        }
> +        if (buf[0] == '\0') {
> +            fail_reason = "Missing unix socket path";
> +            goto fail_syntax;
> +        }
> +        if (buf[0] != '/') {
> +            fail_reason = "unix socket path must be absolute";
> +            goto fail_syntax;
> +        }
> +
> +        size_t path_len = strlen(buf);
> +        if (path_len > sizeof(host_addr.un.sun_path) - 1) {
> +            fail_reason = "Unix socket path is too long";
> +            goto fail_syntax;
> +        }
> +
> +        struct stat st;
> +        if (stat(buf, &st) == 0) {

Coverity notes that we do a check on the filename here
with stat()...

> +            if (!S_ISSOCK(st.st_mode)) {
> +                fail_reason = "file exists and it's not unix socket";
> +                goto fail_syntax;
> +            }
> +
> +            if (unlink(buf) < 0) {

...and then later we do an unlink() if it's a unix socket.
But Coverity points out that an attacker could change what
the filename points to between the stat and the unlink,
causing us to unlink some non-socket file.

Do we care ?

> +                error_setg_errno(errp, errno, "Failed to unlink '%s'", buf);
> +                goto fail_syntax;
> +            }
> +        }
> +        host_addr.un.sun_family = AF_UNIX;
> +        memcpy(host_addr.un.sun_path, buf, path_len);
> +        host_addr_size = sizeof(host_addr.un);
> +    } else

thanks
-- PMM


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

* Re: [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket
  2025-10-23  9:58   ` Peter Maydell
@ 2025-10-23 10:10     ` Samuel Thibault
  2025-10-23 10:38       ` Peter Maydell
  0 siblings, 1 reply; 6+ messages in thread
From: Samuel Thibault @ 2025-10-23 10:10 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Viktor Kurilko, stefanha, jan.kiszka

Hello,

Peter Maydell, le jeu. 23 oct. 2025 10:58:10 +0100, a ecrit:
> Coverity worries here about a possible time-of-check-time-of-use
> bug (CID 1641394). This is a heuristic that tends to fire even
> when there's no interesting attack possible, but I don't
> know what this code is doing so I raise it here:
> 
> > +#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
> > +        struct stat st;
> > +        if (stat(buf, &st) == 0) {
> 
> Coverity notes that we do a check on the filename here
> with stat()...
> 
> > +            if (!S_ISSOCK(st.st_mode)) {
> > +                fail_reason = "file exists and it's not unix socket";
> > +                goto fail_syntax;
> > +            }
> > +
> > +            if (unlink(buf) < 0) {
> 
> ...and then later we do an unlink() if it's a unix socket.
> But Coverity points out that an attacker could change what
> the filename points to between the stat and the unlink,
> causing us to unlink some non-socket file.
> 
> Do we care ?

It is true that an "attacker" could be pointing qemu to a symlink to
a socket, and stat() will follow it, and they can change the symlink
into something else (a symlink to something else, or another type of
file), and we'll unlink() that (which will not follow any symlink, so
just unlink the given path).

I don't see which harmful scenario we could have here. Either the
attacker has control over the given path, and we'll just unlink it, too
bad for them, or they don't have control over the given path, and they
won't be able to change it to their liking between stat() and unlink().

Once thing we could do is to use fstatat to pass AT_SYMLINK_NOFOLLOW,
(when fstatat is available), but I'm not really seeing it worth the
effort.

Samuel


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

* Re: [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket
  2025-10-23 10:10     ` Samuel Thibault
@ 2025-10-23 10:38       ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2025-10-23 10:38 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: qemu-devel, Viktor Kurilko, stefanha, jan.kiszka

On Thu, 23 Oct 2025 at 11:12, Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Hello,
>
> Peter Maydell, le jeu. 23 oct. 2025 10:58:10 +0100, a ecrit:
> > Coverity worries here about a possible time-of-check-time-of-use
> > bug (CID 1641394). This is a heuristic that tends to fire even
> > when there's no interesting attack possible, but I don't
> > know what this code is doing so I raise it here:
> >
> > > +#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
> > > +        struct stat st;
> > > +        if (stat(buf, &st) == 0) {
> >
> > Coverity notes that we do a check on the filename here
> > with stat()...
> >
> > > +            if (!S_ISSOCK(st.st_mode)) {
> > > +                fail_reason = "file exists and it's not unix socket";
> > > +                goto fail_syntax;
> > > +            }
> > > +
> > > +            if (unlink(buf) < 0) {
> >
> > ...and then later we do an unlink() if it's a unix socket.
> > But Coverity points out that an attacker could change what
> > the filename points to between the stat and the unlink,
> > causing us to unlink some non-socket file.
> >
> > Do we care ?
>
> It is true that an "attacker" could be pointing qemu to a symlink to
> a socket, and stat() will follow it, and they can change the symlink
> into something else (a symlink to something else, or another type of
> file), and we'll unlink() that (which will not follow any symlink, so
> just unlink the given path).
>
> I don't see which harmful scenario we could have here. Either the
> attacker has control over the given path, and we'll just unlink it, too
> bad for them, or they don't have control over the given path, and they
> won't be able to change it to their liking between stat() and unlink().

Yes, I agree. The "only unlink if it was a unix socket"
check is essentially a molly-guard against a user passing
a wrong filename. If an attacker bypasses the molly-guard
this doesn't gain them anything. Similarly, an attacker
reinstating something at the file-path after the unlink()
but before slirp binds the unix socket is pretty futile
as it will just cause the bind to fail.

I'll mark the coverity issue as a false-positive.

-- PMM


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

end of thread, other threads:[~2025-10-23 10:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-05 19:47 [PULL 0/1] slirp branch Samuel Thibault
2025-10-05 19:47 ` [PULL 1/1] Add a feature for mapping a host unix socket to a guest tcp socket Samuel Thibault
2025-10-23  9:58   ` Peter Maydell
2025-10-23 10:10     ` Samuel Thibault
2025-10-23 10:38       ` Peter Maydell
2025-10-06 21:59 ` [PULL 0/1] slirp branch Richard Henderson

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).