Re: [libvirt] PATCH: 19/25: Remove use of non-threadsafe POSIX apis

On Fri, Jan 16, 2009 at 12:25:02PM +0000, Daniel P. Berrange wrote: Here's an update with that usage in it 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 @@ -688,20 +688,18 @@ static int qemudInitPaths(struct qemud_s if (snprintf(server->logDir, PATH_MAX, "%s/log/libvirt/", LOCAL_STATE_DIR) >= PATH_MAX) goto snprintf_error; } else { - struct passwd *pw; + char *userdir = virGetUserDirectory(NULL, uid); - if (!(pw = getpwuid(uid))) { - VIR_ERROR(_("Failed to find user record for uid '%d': %s"), - uid, strerror(errno)); - return -1; + if (snprintf(sockname, maxlen, &quot;(a)%s/.libvirt/libvirt-sock&quot;, userdir) >= maxlen) { + VIR_FREE(userdir); + goto snprintf_error; } - if (snprintf(sockname, maxlen, &quot;(a)%s/.libvirt/libvirt-sock&quot;, pw->pw_dir) >= maxlen) + if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log", userdir) >= PATH_MAX) { + VIR_FREE(userdir); goto snprintf_error; - - if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log", pw->pw_dir) >= PATH_MAX) - goto snprintf_error; - + } + VIR_FREE(userdir); } /* !remote */ return 0; @@ -2433,8 +2431,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/libvirt_private.syms b/src/libvirt_private.syms --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -311,6 +311,7 @@ virAsprintf; virRun; virSkipSpaces; virKillProcess; +virGetUserDirectory; # uuid.h 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 @@ -195,7 +195,6 @@ networkAutostartConfigs(struct network_d static int networkStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; int err; @@ -216,19 +215,22 @@ networkStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { - networkLog(NETWORK_ERR, _("Failed to find user record for uid '%d': %s\n"), - uid, strerror(errno)); + char *userdir = virGetUserDirectory(NULL, uid); + + if (!userdir) + goto error; + + if (virAsprintf(&driverState->logDir, + "%s/.libvirt/qemu/log", userdir) == -1) { + VIR_FREE(userdir); goto out_of_memory; } - if (virAsprintf(&driverState->logDir, - "%s/.libvirt/qemu/log", pw->pw_dir) == -1) - goto out_of_memory; - - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) { + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + VIR_FREE(userdir); goto out_of_memory; } + VIR_FREE(userdir); } /* Configuration paths are either ~/.libvirt/qemu/... (session) or 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 @@ -382,7 +382,6 @@ next: static int qemudStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; @@ -421,27 +420,30 @@ qemudStartup(void) { "%s/run/libvirt/qemu/", LOCAL_STATE_DIR) == -1) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { - qemudLog(QEMUD_ERR, _("Failed to find user record for uid '%d': %s\n"), - uid, strerror(errno)); - goto error; - } + char *userdir = virGetUserDirectory(NULL, uid); + if (!userdir) + goto error; if (virAsprintf(&qemu_driver->logDir, - "%s/.libvirt/qemu/log", pw->pw_dir) == -1) - goto out_of_memory; - - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) - goto out_of_memory; + "%s/.libvirt/qemu/log", userdir) == -1) { + VIR_FREE(userdir); + goto out_of_memory; + } + + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + VIR_FREE(userdir); + goto out_of_memory; + } + VIR_FREE(userdir); if (virAsprintf(&qemu_driver->stateDir, "%s/qemu/run", base) == -1) goto out_of_memory; } if (virFileMakePath(qemu_driver->stateDir) < 0) { - qemudLog(QEMUD_ERR, _("Failed to create state dir '%s': %s\n"), - qemu_driver->stateDir, strerror(errno)); - goto error; + qemudLog(QEMUD_ERR, _("Failed to create state dir '%s': %s\n"), + qemu_driver->stateDir, strerror(errno)); + goto error; } /* Configuration paths are either ~/.libvirt/qemu/... (session) or diff --git a/src/remote_internal.c b/src/remote_internal.c --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -605,19 +605,16 @@ doRemoteOpen (virConnectPtr conn, case trans_unix: { if (!sockname) { if (flags & VIR_DRV_OPEN_REMOTE_USER) { - struct passwd *pw; - uid_t uid = getuid(); - - if (!(pw = getpwuid(uid))) { - virReportSystemError(conn, errno, - _("unable to lookup user '%d'"), - uid); + char *userdir = virGetUserDirectory(conn, getuid()); + + if (!userdir) goto failed; + + if (virAsprintf(&sockname, "@%s" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0) { + VIR_FREE(userdir); + goto out_of_memory; } - - if (virAsprintf(&sockname, "@%s" LIBVIRTD_USER_UNIX_SOCKET, pw->pw_dir) < 0) - goto out_of_memory; - + VIR_FREE(userdir); } else { if (flags & VIR_DRV_OPEN_REMOTE_RO) sockname = strdup (LIBVIRTD_PRIV_UNIX_SOCKET_RO); 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,16 +124,17 @@ storageDriverStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { - storageLog("Failed to find user record for uid '%d': %s", - uid, strerror(errno)); + char *userdir = virGetUserDirectory(NULL, uid); + + if (!userdir) + goto error; + + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + storageLog("out of memory in virAsprintf"); + VIR_FREE(userdir); goto out_of_memory; } - - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) { - storageLog("out of memory in virAsprintf"); - goto out_of_memory; - } + VIR_FREE(userdir); } /* Configuration paths are either ~/.libvirt/storage/... (session) or 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,9 @@ cleanup: static int umlStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; + char *userdir = NULL; if (VIR_ALLOC(uml_driver) < 0) return -1; @@ -325,11 +325,9 @@ umlStartup(void) { /* Don't have a dom0 so start from 1 */ uml_driver->nextvmid = 1; - if (!(pw = getpwuid(uid))) { - umlLog(VIR_LOG_ERROR, _("Failed to find user record for uid '%d': %s\n"), - uid, strerror(errno)); + userdir = virGetUserDirectory(NULL, uid); + if (!userdir) goto error; - } if (!uid) { if (virAsprintf(&uml_driver->logDir, @@ -339,16 +337,17 @@ umlStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { + if (virAsprintf(&uml_driver->logDir, - "%s/.libvirt/uml/log", pw->pw_dir) == -1) + "%s/.libvirt/uml/log", userdir) == -1) goto out_of_memory; - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) goto out_of_memory; } if (virAsprintf(&uml_driver->monitorDir, - "%s/.uml", pw->pw_dir) == -1) + "%s/.uml", userdir) == -1) goto out_of_memory; /* Configuration paths are either ~/.libvirt/uml/... (session) or @@ -403,6 +402,8 @@ umlStartup(void) { umlAutostartConfigs(uml_driver); umlDriverUnlock(uml_driver); + VIR_FREE(userdir); + return 0; out_of_memory: @@ -410,6 +411,7 @@ out_of_memory: "%s", _("umlStartup: out of memory\n")); error: + VIR_FREE(userdir); VIR_FREE(base); umlDriverUnlock(uml_driver); umlShutdown(); diff --git a/src/util.c b/src/util.c --- a/src/util.c +++ b/src/util.c @@ -49,6 +49,9 @@ #include <paths.h> #endif #include <netdb.h> +#ifdef HAVE_GETPWUID_R +#include <pwd.h> +#endif #include "virterror_internal.h" #include "logging.h" @@ -1431,3 +1434,37 @@ int virKillProcess(pid_t pid, int sig) return kill(pid, sig); #endif } + + +#ifdef HAVE_GETPWUID_R +char *virGetUserDirectory(virConnectPtr conn, + uid_t uid) +{ + char *strbuf; + char *ret; + struct passwd pwbuf; + struct passwd *pw; + size_t strbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); + + if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { + virReportOOMError(conn); + return NULL; + } + + if (getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw) != 0) { + virReportSystemError(conn, errno, + _("Failed to find user record for uid '%d'"), + uid); + VIR_FREE(strbuf); + return NULL; + } + + ret = strdup(pw->pw_dir); + + VIR_FREE(strbuf); + if (!ret) + virReportOOMError(conn); + + return NULL; +} +#endif diff --git a/src/util.h b/src/util.h --- a/src/util.h +++ b/src/util.h @@ -172,4 +172,9 @@ char *virGetHostname(void); int virKillProcess(pid_t pid, int sig); +#ifdef HAVE_GETPWUID_R +char *virGetUserDirectory(virConnectPtr conn, + uid_t uid); +#endif + #endif /* __VIR_UTIL_H__ */ diff --git a/src/xen_unified.c b/src/xen_unified.c --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -278,9 +278,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 @@ -136,13 +136,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; struct xs_handle *xshandle; /* handle to talk to the xenstore */ diff --git a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -69,30 +69,6 @@ #define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 3 #endif -/** - * 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 @@ -140,7 +116,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")); @@ -154,7 +130,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; @@ -767,18 +743,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); } @@ -795,38 +769,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; } @@ -2721,14 +2728,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 :|