1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/5] Fix handling of IPv4/IPv6 dual stack
  4. View patch

[Qemu-devel] [PATCH v2 2/5] sockets: don't block IPv4 clients when listening on "::"

Daniel P. Berrange posted 5 patches 8 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 2/5] sockets: don't block IPv4 clients when listening on "::"
Posted by Daniel P. Berrange 8 years, 8 months ago 
When inet_parse() parses the hostname, it is forcing the
has_ipv6 && ipv6 flags if the address contains a ":". This
means that if the user had set the ipv4=on flag, to try to
restrict the listener to just ipv4, an error would not have
been raised.  eg

   -incoming tcp:[::]:9000,ipv4

should have raised an error because listening for IPv4
on "::" is a non-sensical combination. With this removed,
we now call getaddrinfo() on "::" passing PF_INET and
so getaddrinfo reports an error about the hostname being
incompatible with the requested protocol.

Likewise it is explicitly setting the has_ipv4 & ipv4
flags when the address contains only digits + '.'. This
has no ill-effect, but also has no benefit, so is removed.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 util/qemu-sockets.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 397212b..b82412e 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -618,16 +618,12 @@ int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
             error_setg(errp, "error parsing IPv6 address '%s'", str);
             return -1;
         }
-        addr->ipv6 = addr->has_ipv6 = true;
     } else {
         /* hostname or IPv4 addr */
         if (sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos) != 2) {
             error_setg(errp, "error parsing address '%s'", str);
             return -1;
         }
-        if (host[strspn(host, "0123456789.")] == '\0') {
-            addr->ipv4 = addr->has_ipv4 = true;
-        }
     }
 
     addr->host = g_strdup(host);
-- 
2.9.3
Re: [Qemu-devel] [PATCH v2 2/5] sockets: don't block IPv4 clients when listening on "::"
Posted by Philippe Mathieu-Daudé 8 years, 8 months ago 
On 05/19/2017 03:03 PM, Daniel P. Berrange wrote:
> When inet_parse() parses the hostname, it is forcing the
> has_ipv6 && ipv6 flags if the address contains a ":". This
> means that if the user had set the ipv4=on flag, to try to
> restrict the listener to just ipv4, an error would not have
> been raised.  eg
>
>    -incoming tcp:[::]:9000,ipv4
>
> should have raised an error because listening for IPv4
> on "::" is a non-sensical combination. With this removed,
> we now call getaddrinfo() on "::" passing PF_INET and
> so getaddrinfo reports an error about the hostname being
> incompatible with the requested protocol.
>
> Likewise it is explicitly setting the has_ipv4 & ipv4
> flags when the address contains only digits + '.'. This
> has no ill-effect, but also has no benefit, so is removed.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  util/qemu-sockets.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 397212b..b82412e 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -618,16 +618,12 @@ int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
>              error_setg(errp, "error parsing IPv6 address '%s'", str);
>              return -1;
>          }
> -        addr->ipv6 = addr->has_ipv6 = true;
>      } else {
>          /* hostname or IPv4 addr */
>          if (sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos) != 2) {
>              error_setg(errp, "error parsing address '%s'", str);
>              return -1;
>          }
> -        if (host[strspn(host, "0123456789.")] == '\0') {
> -            addr->ipv4 = addr->has_ipv4 = true;
> -        }
>      }
>
>      addr->host = g_strdup(host);
>
Re: [Qemu-devel] [PATCH v2 2/5] sockets: don't block IPv4 clients when listening on "::"
Posted by Eric Blake 8 years, 8 months ago 
On 05/19/2017 01:03 PM, Daniel P. Berrange wrote:
> When inet_parse() parses the hostname, it is forcing the
> has_ipv6 && ipv6 flags if the address contains a ":". This
> means that if the user had set the ipv4=on flag, to try to
> restrict the listener to just ipv4, an error would not have
> been raised.  eg
> 
>    -incoming tcp:[::]:9000,ipv4
> 
> should have raised an error because listening for IPv4
> on "::" is a non-sensical combination.

If I understand correctly, the correct response (and post-patch
behavior) is an error (mismatch between requesting IPv4-only usage while
giving an IPv6 address), but the buggy response (pre-patch behavior) is
that we ended up setting ipv6 in addition to the user-set ipv4 (because
we found a ':'), and then end up listening on IPv6 after all contrary to
the user's request.


> With this removed,
> we now call getaddrinfo() on "::" passing PF_INET and
> so getaddrinfo reports an error about the hostname being
> incompatible with the requested protocol.
> 
> Likewise it is explicitly setting the has_ipv4 & ipv4
> flags when the address contains only digits + '.'. This
> has no ill-effect, but also has no benefit, so is removed.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  util/qemu-sockets.c | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2 2/5] sockets: don't block IPv4 clients when listening on "::"
Posted by Daniel P. Berrange 8 years, 8 months ago 
On Mon, May 22, 2017 at 10:30:55AM -0500, Eric Blake wrote:
> On 05/19/2017 01:03 PM, Daniel P. Berrange wrote:
> > When inet_parse() parses the hostname, it is forcing the
> > has_ipv6 && ipv6 flags if the address contains a ":". This
> > means that if the user had set the ipv4=on flag, to try to
> > restrict the listener to just ipv4, an error would not have
> > been raised.  eg
> > 
> >    -incoming tcp:[::]:9000,ipv4
> > 
> > should have raised an error because listening for IPv4
> > on "::" is a non-sensical combination.
> 
> If I understand correctly, the correct response (and post-patch
> behavior) is an error (mismatch between requesting IPv4-only usage while
> giving an IPv6 address), but the buggy response (pre-patch behavior) is
> that we ended up setting ipv6 in addition to the user-set ipv4 (because
> we found a ':'), and then end up listening on IPv6 after all contrary to
> the user's request.

Yes, with this patch you get this error:

  qemu-system-x86_64: -incoming tcp:[::]:9000,ipv4: address resolution
    failed for :::9000: Address family for hostname not supported

because it now (correctly) tries to resolve "::" as an IPv4 address
and fails (just like -chardev and -vnc already do)

> 
> 
> > With this removed,
> > we now call getaddrinfo() on "::" passing PF_INET and
> > so getaddrinfo reports an error about the hostname being
> > incompatible with the requested protocol.
> > 
> > Likewise it is explicitly setting the has_ipv4 & ipv4
> > flags when the address contains only digits + '.'. This
> > has no ill-effect, but also has no benefit, so is removed.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  util/qemu-sockets.c | 4 ----
> >  1 file changed, 4 deletions(-)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 




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 :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.