| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <874kzfbybk.fsf@vitty.brq.redhat.com>
Date: Thu, 07 Nov 2019 14:39:11 +0100
From: Vitaly Kuznetsov <vkuznets@...hat.com>
To: Olaf Hering <olaf@...fle.de>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
Sasha Levin <sashal@...nel.org>,
"open list\:Hyper-V CORE AND DRIVERS" <linux-hyperv@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Cc: Olaf Hering <olaf@...fle.de>
Subject: Re: [PATCH v1] tools/hv: async name resolution in kvp_daemon
Olaf Hering <olaf@...fle.de> writes:
> The hostname is resolved just once since commit 58125210ab3b ("Tools:
> hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
> responds within the timeout limits to requests from the host.
>
> If for some reason getaddrinfo fails, the string returned by the
> "FullyQualifiedDomainName" request contains some error string, which is
> then used by tools on the host side.
>
> Adjust the code to resolve the current hostname in a separate thread.
> This thread loops until getaddrinfo returns success. During this time
> all "FullyQualifiedDomainName" requests will be answered with an empty
> string.
>
> Signed-off-by: Olaf Hering <olaf@...fle.de>
> ---
> tools/hv/Makefile | 2 ++
> tools/hv/hv_kvp_daemon.c | 69 ++++++++++++++++++++++++++++++++----------------
> 2 files changed, 48 insertions(+), 23 deletions(-)
>
> diff --git a/tools/hv/Makefile b/tools/hv/Makefile
> index b57143d9459c..3b5481015a84 100644
> --- a/tools/hv/Makefile
> +++ b/tools/hv/Makefile
> @@ -22,6 +22,8 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
>
> ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
>
> +$(OUTPUT)hv_kvp_daemon: LDFLAGS += -lpthread
> +
> all: $(ALL_PROGRAMS)
>
> export srctree OUTPUT CC LD CFLAGS
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index e9ef4ca6a655..22cf1c4dbf5c 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -41,6 +41,7 @@
> #include <net/if.h>
> #include <limits.h>
> #include <getopt.h>
> +#include <pthread.h>
>
> /*
> * KVP protocol: The user mode component first registers with the
> @@ -85,7 +86,7 @@ static char *processor_arch;
> static char *os_build;
> static char *os_version;
> static char *lic_version = "Unknown version";
> -static char full_domain_name[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
> +static char *full_domain_name;
> static struct utsname uts_buf;
>
> /*
> @@ -1327,27 +1328,53 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> return error;
> }
>
> -
> -static void
> -kvp_get_domain_name(char *buffer, int length)
> +/*
> + * Async retrival of Fully Qualified Domain Name because getaddrinfo takes an
> + * unpredictable amount of time to finish.
> + */
> +static void *kvp_getaddrinfo(void *p)
> {
> - struct addrinfo hints, *info ;
> - int error = 0;
> + char *tmp, **str_ptr = (char **)p;
> + char hostname[HOST_NAME_MAX + 1];
> + struct addrinfo *info, hints = {
> + .ai_family = AF_INET, /* Get only ipv4 addrinfo. */
> + .ai_socktype = SOCK_STREAM,
> + .ai_flags = AI_CANONNAME,
> + };
> + int ret;
> +
> + if (gethostname(hostname, sizeof(hostname) - 1) < 0)
> + goto out;
> +
> + do {
> + ret = getaddrinfo(hostname, NULL, &hints, &info);
> + if (ret)
> + sleep(1);
Is it only EAI_AGAIN or do you see any other return values which justify
the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
be infinitely looping with EAI_FAIL.
> + } while (ret);
> +
> + ret = asprintf(&tmp, "%s", info->ai_canonname);
> + freeaddrinfo(info);
> + if (ret <= 0)
> + goto out;
> +
> + if (ret > HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
> + tmp[HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1] = '\0';
> + *str_ptr = tmp;
>
> - gethostname(buffer, length);
> - memset(&hints, 0, sizeof(hints));
> - hints.ai_family = AF_INET; /*Get only ipv4 addrinfo. */
> - hints.ai_socktype = SOCK_STREAM;
> - hints.ai_flags = AI_CANONNAME;
> +out:
> + pthread_exit(NULL);
> +}
> +
> +static void kvp_obtain_domain_name(char **str_ptr)
> +{
> + pthread_t t;
>
> - error = getaddrinfo(buffer, NULL, &hints, &info);
> - if (error != 0) {
> - snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
> - error, gai_strerror(error));
> + if (pthread_create(&t, NULL, kvp_getaddrinfo, str_ptr)) {
> + syslog(LOG_ERR, "pthread_create failed; error: %d %s",
> + errno, strerror(errno));
> return;
> }
> - snprintf(buffer, length, "%s", info->ai_canonname);
> - freeaddrinfo(info);
> + pthread_detach(t);
I think this should be complemented with pthread_cancel/pthread_join
before exiting main().
> }
>
> void print_usage(char *argv[])
> @@ -1412,11 +1439,7 @@ int main(int argc, char *argv[])
> * Retrieve OS release information.
> */
> kvp_get_os_info();
> - /*
> - * Cache Fully Qualified Domain Name because getaddrinfo takes an
> - * unpredictable amount of time to finish.
> - */
> - kvp_get_domain_name(full_domain_name, sizeof(full_domain_name));
> + kvp_obtain_domain_name(&full_domain_name);
>
> if (kvp_file_init()) {
> syslog(LOG_ERR, "Failed to initialize the pools");
> @@ -1571,7 +1594,7 @@ int main(int argc, char *argv[])
>
> switch (hv_msg->body.kvp_enum_data.index) {
> case FullyQualifiedDomainName:
> - strcpy(key_value, full_domain_name);
> + strcpy(key_value, full_domain_name ? : "");
> strcpy(key_name, "FullyQualifiedDomainName");
> break;
> case IntegrationServicesVersion:
--
Vitaly
Powered by blists - more mailing lists