Remove use of various non thread safe functions, specifically strtok getmntent getgrnam getpwuid gethostbyname I thought we also had to remove use of readdir(), but it turns out that *is* threadsafe provided you only use each DIR* object from one thread at a time, which is fine for our needs. The readdir_r() function is absolutely horrific to use safely so its just as well we don't need to. For the gethostbyname removal in Xen, I took the opportunity to switch to getaddrinfo(), since even gethostbyname_r is deprecated these days - not IPv6 aware. configure.in | 2 proxy/libvirt_proxy.c | 3 qemud/qemud.c | 9 +- src/lxc_container.c | 9 +- src/network_driver.c | 7 +- src/openvz_driver.c | 3 src/qemu_driver.c | 7 +- src/remote_internal.c | 4 - src/storage_backend_fs.c | 7 +- src/storage_driver.c | 7 +- src/uml_driver.c | 6 + src/xen_unified.c | 3 src/xen_unified.h | 10 +-- src/xend_internal.c | 147 +++++++++++++++++++++++++---------------------- 14 files changed, 123 insertions(+), 101 deletions(-) Daniel diff --git a/configure.in b/configure.in --- a/configure.in +++ b/configure.in @@ -75,7 +75,7 @@ dnl Availability of various common funct AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid]) dnl Availability of various not common threadsafe functions -AC_CHECK_FUNCS([strerror_r]) +AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h syslog.h]) diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -76,9 +76,6 @@ proxyInitXen(void) { priv->handle = -1; priv->xendConfigVersion = -1; - priv->type = -1; - priv->len = -1; - priv->addr = NULL; priv->xshandle = NULL; priv->proxy = -1; diff --git a/qemud/qemud.c b/qemud/qemud.c --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -689,9 +689,11 @@ static int qemudInitPaths(struct qemud_s if (snprintf(server->logDir, PATH_MAX, "%s/log/libvirt/", LOCAL_STATE_DIR) >= PATH_MAX) goto snprintf_error; } else { + char buf[1024]; + struct passwd pwbuf; struct passwd *pw; - if (!(pw = getpwuid(uid))) { + if (getpwuid_r(uid, &pwbuf, buf, sizeof(buf), &pw) != 0) { VIR_ERROR(_("Failed to find user record for uid '%d': %s"), uid, strerror(errno)); return -1; @@ -2376,8 +2378,9 @@ remoteReadConfigFile (struct qemud_serve if (getuid() != 0) { VIR_WARN0(_("Cannot set group when not running as root")); } else { - struct group *grp = getgrnam(unix_sock_group); - if (!grp) { + char buf[1024]; + struct group grpdata, *grp; + if (getgrnam_r(unix_sock_group, &grpdata, buf, sizeof(buf), &grp) != 0 || !grp) { VIR_ERROR(_("Failed to lookup group '%s'"), unix_sock_group); goto free_and_fail; } diff --git a/src/lxc_container.c b/src/lxc_container.c --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -414,19 +414,20 @@ static int lxcContainerMountNewFS(virDom static int lxcContainerUnmountOldFS(void) { - struct mntent *mntent; + struct mntent mntent; char **mounts = NULL; int nmounts = 0; FILE *procmnt; int i; + char mntbuf[1024]; if (!(procmnt = setmntent("/proc/mounts", "r"))) { virReportSystemError(NULL, errno, "%s", _("failed to read /proc/mounts")); return -1; } - while ((mntent = getmntent(procmnt)) != NULL) { - if (!STRPREFIX(mntent->mnt_dir, "/.oldroot")) + while (getmntent_r(procmnt, &mntent, mntbuf, sizeof(mntbuf)) != NULL) { + if (!STRPREFIX(mntent.mnt_dir, "/.oldroot")) continue; if (VIR_REALLOC_N(mounts, nmounts+1) < 0) { @@ -434,7 +435,7 @@ static int lxcContainerUnmountOldFS(void lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return -1; } - if (!(mounts[nmounts++] = strdup(mntent->mnt_dir))) { + if (!(mounts[nmounts++] = strdup(mntent.mnt_dir))) { endmntent(procmnt); lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return -1; diff --git a/src/network_driver.c b/src/network_driver.c --- a/src/network_driver.c +++ b/src/network_driver.c @@ -131,7 +131,6 @@ networkAutostartConfigs(struct network_d static int networkStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; if (VIR_ALLOC(driverState) < 0) @@ -151,7 +150,11 @@ networkStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { + char buf[1024]; + struct passwd pwbuf; + struct passwd *pw; + + if (getpwuid_r(uid, &pwbuf, buf, sizeof(buf), &pw) != 0) { networkLog(NETWORK_ERR, _("Failed to find user record for uid '%d': %s\n"), uid, strerror(errno)); goto out_of_memory; diff --git a/src/openvz_driver.c b/src/openvz_driver.c --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -448,11 +448,12 @@ openvzGenerateContainerVethName(int veid if ( (ret = openvzReadConfigParam(veid, "NETIF", temp, sizeof(temp))) <= 0) { snprintf(temp, sizeof(temp), "eth0"); } else { + char *saveptr; char *s; int max = 0; /* get maximum interface number (actually, it is the last one) */ - for (s=strtok(temp, ";"); s; s=strtok(NULL, ";")) { + for (s=strtok_r(temp, ";", &saveptr); s; s=strtok_r(NULL, ";", &saveptr)) { int x; if (sscanf(s, "ifname=eth%d", &x) != 1) return NULL; diff --git a/src/qemu_driver.c b/src/qemu_driver.c --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -265,7 +265,6 @@ cleanup: static int qemudStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; @@ -304,7 +303,11 @@ qemudStartup(void) { "%s/run/libvirt/qemu/", LOCAL_STATE_DIR) == -1) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { + char buf[1024]; + struct passwd pwbuf; + struct passwd *pw; + + if (getpwuid_r(uid, &pwbuf, buf, sizeof(buf), &pw) != 0) { qemudLog(QEMUD_ERR, _("Failed to find user record for uid '%d': %s\n"), uid, strerror(errno)); goto error; diff --git a/src/remote_internal.c b/src/remote_internal.c --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -603,10 +603,12 @@ doRemoteOpen (virConnectPtr conn, case trans_unix: { if (!sockname) { if (flags & VIR_DRV_OPEN_REMOTE_USER) { + char buf[1024]; + struct passwd pwbuf; struct passwd *pw; uid_t uid = getuid(); - if (!(pw = getpwuid(uid))) { + if (getpwuid_r(uid, &pwbuf, buf, sizeof(buf), &pw) != 0) { virReportSystemError(conn, errno, _("unable to lookup user '%d'"), uid); diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -385,7 +385,8 @@ static int virStorageBackendFileSystemIsMounted(virConnectPtr conn, virStoragePoolObjPtr pool) { FILE *mtab; - struct mntent *ent; + struct mntent ent; + char buf[1024]; if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) { virReportSystemError(conn, errno, @@ -394,8 +395,8 @@ virStorageBackendFileSystemIsMounted(vir return -1; } - while ((ent = getmntent(mtab)) != NULL) { - if (STREQ(ent->mnt_dir, pool->def->target.path)) { + while ((getmntent_r(mtab, &ent, buf, sizeof(buf))) != NULL) { + if (STREQ(ent.mnt_dir, pool->def->target.path)) { fclose(mtab); return 1; } diff --git a/src/storage_driver.c b/src/storage_driver.c --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -108,7 +108,6 @@ storageDriverAutostart(virStorageDriverS static int storageDriverStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; @@ -125,7 +124,11 @@ storageDriverStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { + char buf[1024]; + struct passwd pwbuf; + struct passwd *pw; + + if (getpwuid_r(uid, &pwbuf, buf, sizeof(buf), &pw) != 0) { storageLog("Failed to find user record for uid '%d': %s", uid, strerror(errno)); goto out_of_memory; diff --git a/src/uml_driver.c b/src/uml_driver.c --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -309,9 +309,11 @@ cleanup: static int umlStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; + char buf[1024]; + struct passwd pwbuf; + struct passwd *pw; if (VIR_ALLOC(uml_driver) < 0) return -1; @@ -325,7 +327,7 @@ umlStartup(void) { /* Don't have a dom0 so start from 1 */ uml_driver->nextvmid = 1; - if (!(pw = getpwuid(uid))) { + if (getpwuid_r(uid, &pwbuf, buf, sizeof(buf), &pw) != 0) { umlLog(VIR_LOG_ERROR, _("Failed to find user record for uid '%d': %s\n"), uid, strerror(errno)); goto error; diff --git a/src/xen_unified.c b/src/xen_unified.c --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -258,9 +258,6 @@ xenUnifiedOpen (virConnectPtr conn, virC priv->handle = -1; priv->xendConfigVersion = -1; - priv->type = -1; - priv->len = -1; - priv->addr = NULL; priv->xshandle = NULL; priv->proxy = -1; diff --git a/src/xen_unified.h b/src/xen_unified.h --- a/src/xen_unified.h +++ b/src/xen_unified.h @@ -142,13 +142,11 @@ struct _xenUnifiedPrivate { int xendConfigVersion; /* XenD config version */ - /* XXX This code is not IPv6 aware. */ /* connection to xend */ - int type; /* PF_UNIX or PF_INET */ - int len; /* length of addr */ - struct sockaddr *addr; /* type of address used */ - struct sockaddr_un addr_un; /* the unix address */ - struct sockaddr_in addr_in; /* the inet address */ + struct sockaddr_storage addr; + socklen_t addrlen; + int addrfamily; + int addrprotocol; /* Keep track of the drivers which opened. We keep a yes/no flag * here for each driver, corresponding to the array drivers in diff --git a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -61,30 +61,6 @@ #endif /* PROXY */ -/** - * xend_connection_type: - * - * The connection to the Xen Daemon can be done either though a normal TCP - * socket or a local domain direct connection. - */ -enum xend_connection_type { - XEND_DOMAIN, - XEND_TCP, -}; - -/** - * xend: - * - * Structure associated to a connection to a Xen daemon - */ -struct xend { - int len; - int type; - struct sockaddr *addr; - struct sockaddr_un addr_un; - struct sockaddr_in addr_in; -}; - #ifndef PROXY static int @@ -132,7 +108,7 @@ do_connect(virConnectPtr xend) int no_slow_start = 1; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) xend->privateData; - s = socket(priv->type, SOCK_STREAM, 0); + s = socket(priv->addrfamily, SOCK_STREAM, priv->addrprotocol); if (s == -1) { virXendError(xend, VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create a socket")); @@ -146,7 +122,7 @@ do_connect(virConnectPtr xend) sizeof(no_slow_start)); - if (connect(s, priv->addr, priv->len) == -1) { + if (connect(s, (struct sockaddr *)&priv->addr, priv->addrlen) == -1) { serrno = errno; close(s); errno = serrno; @@ -804,18 +780,16 @@ xenDaemonOpen_unix(virConnectPtr conn, c if ((conn == NULL) || (path == NULL)) return (-1); - addr = &priv->addr_un; + memset(&priv->addr, 0, sizeof(priv->addr)); + priv->addrfamily = AF_UNIX; + priv->addrprotocol = PF_UNIX; + priv->addrlen = sizeof(struct sockaddr_un); + + addr = (struct sockaddr_un *)&priv->addr; addr->sun_family = AF_UNIX; memset(addr->sun_path, 0, sizeof(addr->sun_path)); strncpy(addr->sun_path, path, sizeof(addr->sun_path)); - priv->len = sizeof(addr->sun_family) + strlen(addr->sun_path); - if ((unsigned int) priv->len > sizeof(addr->sun_path)) - priv->len = sizeof(addr->sun_path); - - priv->addr = (struct sockaddr *) addr; - priv->type = PF_UNIX; - return (0); } @@ -832,38 +806,71 @@ xenDaemonOpen_unix(virConnectPtr conn, c * Returns 0 in case of success, -1 in case of error. */ static int -xenDaemonOpen_tcp(virConnectPtr conn, const char *host, int port) -{ - struct in_addr ip; - struct hostent *pent; - xenUnifiedPrivatePtr priv; - - if ((conn == NULL) || (host == NULL) || (port <= 0)) - return (-1); - - priv = (xenUnifiedPrivatePtr) conn->privateData; - - pent = gethostbyname(host); - if (pent == NULL) { - if (inet_aton(host, &ip) == 0) { - virXendError(NULL, VIR_ERR_UNKNOWN_HOST, - _("gethostbyname failed: %s"), host); - errno = ESRCH; - return (-1); - } - } else { - memcpy(&ip, pent->h_addr_list[0], sizeof(ip)); - } - - priv->len = sizeof(struct sockaddr_in); - priv->addr = (struct sockaddr *) &priv->addr_in; - priv->type = PF_INET; - - priv->addr_in.sin_family = AF_INET; - priv->addr_in.sin_port = htons(port); - memcpy(&priv->addr_in.sin_addr, &ip, sizeof(ip)); - - return (0); +xenDaemonOpen_tcp(virConnectPtr conn, const char *host, const char *port) +{ + xenUnifiedPrivatePtr priv; + struct addrinfo *res, *r; + struct addrinfo hints; + int saved_errno = EINVAL; + int ret; + + if ((conn == NULL) || (host == NULL) || (port == NULL)) + return (-1); + + priv = (xenUnifiedPrivatePtr) conn->privateData; + + priv->addrlen = 0; + memset(&priv->addr, 0, sizeof(priv->addr)); + + // http://people.redhat.com/drepper/userapi-ipv6.html + memset (&hints, 0, sizeof hints); + hints.ai_socktype = SOCK_STREAM; + hints.ai_flags = AI_ADDRCONFIG; + + ret = getaddrinfo (host, port, &hints, &res); + if (ret != 0) { + virXendError(NULL, VIR_ERR_UNKNOWN_HOST, + _("unable to resolve hostname '%s': %s"), + host, gai_strerror (ret)); + return -1; + } + + /* Try to connect to each returned address in turn. */ + for (r = res; r; r = r->ai_next) { + int sock; + + sock = socket (r->ai_family, SOCK_STREAM, r->ai_protocol); + if (sock == -1) { + saved_errno = errno; + continue; + } + + if (connect (sock, r->ai_addr, r->ai_addrlen) == -1) { + saved_errno = errno; + close (sock); + continue; + } + + priv->addrlen = r->ai_addrlen; + priv->addrfamily = r->ai_family; + priv->addrprotocol = r->ai_protocol; + memcpy(&priv->addr, + r->ai_addr, + r->ai_addrlen); + close(sock); + break; + } + + freeaddrinfo (res); + + if (!priv->addrlen) { + virReportSystemError(conn, saved_errno, + _("unable to connect to '%s:%s'"), + host, port); + return -1; + } + + return 0; } @@ -2765,14 +2772,18 @@ xenDaemonOpen(virConnectPtr conn, /* * try though http on port 8000 */ - ret = xenDaemonOpen_tcp(conn, "localhost", 8000); + ret = xenDaemonOpen_tcp(conn, "localhost", "8000"); if (ret < 0) goto failed; ret = xend_detect_config_version(conn); if (ret == -1) goto failed; } else if (STRCASEEQ (conn->uri->scheme, "http")) { - ret = xenDaemonOpen_tcp(conn, conn->uri->server, conn->uri->port); + char *port; + if (virAsprintf(&port, "%d", conn->uri->port) == -1) + goto failed; + ret = xenDaemonOpen_tcp(conn, conn->uri->server, port); + VIR_FREE(port); if (ret < 0) goto failed; ret = xend_detect_config_version(conn); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list