6:48 a.m.
This imperfection was reported by Andrea here:
https://www.redhat.com/archives/libvir-list/2020-July/msg00753.html
Michal Prívozník (3):
virNetSocketCheckProtocols: Separate out checking family via
getaddrinfo()
virNetSocketCheckProtocols: lookup IPv6 only if suspecting IPv6
virNetSocketCheckProtocols: Confirm IPv4 by lookup too
src/rpc/virnetsocket.c | 67 +++++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 24 deletions(-)
--
2.26.2
6:48 a.m.
New subject: [PATCH 1/3] virNetSocketCheckProtocols: Separate out checking family via getaddrinfo()
The virNetSocketCheckProtocols() function is supposed to tell
caller whether IPv4 and/or IPv6 is supported on the system. In
the initial round, it uses getifaddrs() to see if an interface
has IPv4/IPv6 address assigned and then to double check IPv6 it
uses getaddrinfo() to lookup IPv6 loopback address. Separate out
this latter code because it is going to be reused.
Since the original code lived under an #ifdef and the new
function doesn't it is marked as unused - because on some systems
it may be so.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/rpc/virnetsocket.c | 63 ++++++++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index d1f4c531aa..b6bc3edc3b 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -141,16 +141,48 @@ static int virNetSocketForkDaemon(const char *binary)
}
#endif
+
+static int G_GNUC_UNUSED
+virNetSocketCheckProtocolByLookup(const char *address,
+ int family,
+ bool *hasFamily)
+{
+ struct addrinfo hints;
+ struct addrinfo *ai = NULL;
+ int gaierr;
+
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_family = family;
+ hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
+ hints.ai_socktype = SOCK_STREAM;
+
+ if ((gaierr = getaddrinfo(address, NULL, &hints, &ai)) != 0) {
+ *hasFamily = false;
+
+ if (gaierr == EAI_FAMILY ||
+#ifdef EAI_ADDRFAMILY
+ gaierr == EAI_ADDRFAMILY ||
+#endif
+ gaierr == EAI_NONAME) {
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot resolve ::1 address: %s"),
+ gai_strerror(gaierr));
+ return -1;
+ }
+ } else {
+ *hasFamily = true;
+ }
+
+ freeaddrinfo(ai);
+ return 0;
+}
+
int virNetSocketCheckProtocols(bool *hasIPv4,
bool *hasIPv6)
{
#ifdef HAVE_IFADDRS_H
struct ifaddrs *ifaddr = NULL, *ifa;
- struct addrinfo hints;
- struct addrinfo *ai = NULL;
- int gaierr;
-
- memset(&hints, 0, sizeof(hints));
*hasIPv4 = *hasIPv6 = false;
@@ -172,26 +204,9 @@ int virNetSocketCheckProtocols(bool *hasIPv4,
freeifaddrs(ifaddr);
- hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
- hints.ai_family = AF_INET6;
- hints.ai_socktype = SOCK_STREAM;
- if ((gaierr = getaddrinfo("::1", NULL, &hints, &ai)) != 0) {
- if (gaierr == EAI_FAMILY ||
-# ifdef EAI_ADDRFAMILY
- gaierr == EAI_ADDRFAMILY ||
-# endif
- gaierr == EAI_NONAME) {
- *hasIPv6 = false;
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Cannot resolve ::1 address: %s"),
- gai_strerror(gaierr));
- return -1;
- }
- }
-
- freeaddrinfo(ai);
+ if (virNetSocketCheckProtocolByLookup("::1", AF_INET6, hasIPv6) < 0)
+ return -1;
VIR_DEBUG("Protocols: v4 %d v6 %d", *hasIPv4, *hasIPv6);
--
2.26.2
4:43 p.m.
New subject: [PATCH 1/3] virNetSocketCheckProtocols: Separate out checking family via getaddrinfo()
On a Wednesday in 2020, Michal Privoznik wrote:
The virNetSocketCheckProtocols() function is supposed to tell
caller whether IPv4 and/or IPv6 is supported on the system. In
the initial round, it uses getifaddrs() to see if an interface
has IPv4/IPv6 address assigned and then to double check IPv6 it
uses getaddrinfo() to lookup IPv6 loopback address. Separate out
this latter code because it is going to be reused.
Since the original code lived under an #ifdef and the new
function doesn't it is marked as unused - because on some systems
it may be so.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/rpc/virnetsocket.c | 63 ++++++++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index d1f4c531aa..b6bc3edc3b 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -141,16 +141,48 @@ static int virNetSocketForkDaemon(const char *binary)
}
#endif
+
+static int G_GNUC_UNUSED
+virNetSocketCheckProtocolByLookup(const char *address,
+ int family,
+ bool *hasFamily)
+{
+ struct addrinfo hints;
+ struct addrinfo *ai = NULL;
+ int gaierr;
+
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_family = family;
+ hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
+ hints.ai_socktype = SOCK_STREAM;
+
+ if ((gaierr = getaddrinfo(address, NULL, &hints, &ai)) != 0) {
+ *hasFamily = false;
+
+ if (gaierr == EAI_FAMILY ||
+#ifdef EAI_ADDRFAMILY
+ gaierr == EAI_ADDRFAMILY ||
+#endif
+ gaierr == EAI_NONAME) {
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot resolve ::1 address: %s"),
s/::1/'%s'/
+ gai_strerror(gaierr));
+ return -1;
+ }
+ } else {
+ *hasFamily = true;
+ }
Jano
6:48 a.m.
New subject: [PATCH 2/3] virNetSocketCheckProtocols: lookup IPv6 only if suspecting IPv6
There is not much sense trying to disprove host is IPv6 capable
if we know after first round (getifaddrs()) that is is not.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/rpc/virnetsocket.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b6bc3edc3b..b0d63f0f2c 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -205,7 +205,8 @@ int virNetSocketCheckProtocols(bool *hasIPv4,
freeifaddrs(ifaddr);
- if (virNetSocketCheckProtocolByLookup("::1", AF_INET6, hasIPv6) < 0)
+ if (hasIPv6 &&
+ virNetSocketCheckProtocolByLookup("::1", AF_INET6, hasIPv6) < 0)
return -1;
VIR_DEBUG("Protocols: v4 %d v6 %d", *hasIPv4, *hasIPv6);
--
2.26.2
5:33 a.m.
New subject: [PATCH 2/3] virNetSocketCheckProtocols: lookup IPv6 only if suspecting IPv6
On 7/15/20 7:48 AM, Michal Privoznik wrote:
There is not much sense trying to disprove host is IPv6 capable
if we know after first round (getifaddrs()) that is is not.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/rpc/virnetsocket.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b6bc3edc3b..b0d63f0f2c 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -205,7 +205,8 @@ int virNetSocketCheckProtocols(bool *hasIPv4,
freeifaddrs(ifaddr);
- if (virNetSocketCheckProtocolByLookup("::1", AF_INET6, hasIPv6) < 0)
+ if (hasIPv6 &&
For this and the subsequent hasIPv4 patch, Coverity complains that
checking "if (hasIPv6)" is a REVERSE_INULL issue since a few lines above
we have "*hasIPv4 = *hasIPv6 = false;" or individual setting to true.
This then should be "if (!*hasIPv6..." and of course the same for hasIPv4.
/me wonders what kind of slight optimization exists or anyone cares
about in the ifaddr loop once both has* bool ptrs are proven true - it's
not like there's a set to false...
John
+ virNetSocketCheckProtocolByLookup("::1", AF_INET6,
hasIPv6) < 0)
return -1;
VIR_DEBUG("Protocols: v4 %d v6 %d", *hasIPv4, *hasIPv6);
5:58 a.m.
New subject: [PATCH 2/3] virNetSocketCheckProtocols: lookup IPv6 only if suspecting IPv6
On 7/17/20 12:33 PM, John Ferlan wrote:
On 7/15/20 7:48 AM, Michal Privoznik wrote:
> There is not much sense trying to disprove host is IPv6 capable
> if we know after first round (getifaddrs()) that is is not.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/rpc/virnetsocket.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index b6bc3edc3b..b0d63f0f2c 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -205,7 +205,8 @@ int virNetSocketCheckProtocols(bool *hasIPv4,
> freeifaddrs(ifaddr);
>
>
> - if (virNetSocketCheckProtocolByLookup("::1", AF_INET6, hasIPv6) <
0)
> + if (hasIPv6 &&
For this and the subsequent hasIPv4 patch, Coverity complains that
checking "if (hasIPv6)" is a REVERSE_INULL issue since a few lines above
we have "*hasIPv4 = *hasIPv6 = false;" or individual setting to true.
This then should be "if (!*hasIPv6..." and of course the same for hasIPv4.
/me wonders what kind of slight optimization exists or anyone cares
about in the ifaddr loop once both has* bool ptrs are proven true - it's
not like there's a set to false...
D'oh! It should have been: if (*hasIPv6 && ..); because we are
interested in the value of boolean, not its address. I'll post a patch
shortly.
Michal
6:48 a.m.
New subject: [PATCH 3/3] virNetSocketCheckProtocols: Confirm IPv4 by lookup too
Historically, if we found an interface with an IPv6 address we
did not blindly trust that host is IPv6 capable (as in we can
successfully translate IPv4 addresses) but used getaddrinfo() to
confirm it. Turns out, we have use the same argument for IPv4.
For instance, in an namespace created by the following steps,
getaddrinfo("127.0.0.1", ...) fails (demonstrating by "Socket
TCP/IPv4 Accept" test case failing in virnetsockettest):
unshare -n
ip link set lo up
ip link add dummy0 type dummy
ip link set dummy0 up
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/rpc/virnetsocket.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b0d63f0f2c..c62c2fb3fc 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -204,6 +204,9 @@ int virNetSocketCheckProtocols(bool *hasIPv4,
freeifaddrs(ifaddr);
+ if (hasIPv4 &&
+ virNetSocketCheckProtocolByLookup("127.0.0.1", AF_INET, hasIPv4) <
0)
+ return -1;
if (hasIPv6 &&
virNetSocketCheckProtocolByLookup("::1", AF_INET6, hasIPv6) < 0)
--
2.26.2
4:43 p.m.
On a Wednesday in 2020, Michal Privoznik wrote:
This imperfection was reported by Andrea here:
https://www.redhat.com/archives/libvir-list/2020-July/msg00753.html
Michal Prívozník (3):
virNetSocketCheckProtocols: Separate out checking family via
getaddrinfo()
virNetSocketCheckProtocols: lookup IPv6 only if suspecting IPv6
virNetSocketCheckProtocols: Confirm IPv4 by lookup too
src/rpc/virnetsocket.c | 67 +++++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 24 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1589
days inactive
1591
days old
7 comments
4 participants
participants (4)
-
John Ferlan -
Ján Tomko -
Michal Privoznik -
Michal Prívozník