[libvirt] PATCH: 19/25: Remove use of non-threadsafe POSIX apis
Daniel P. Berrange
berrange at redhat.com
Tue Jan 13 17:46:37 UTC 2009
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 :|
More information about the libvir-list
mailing list