From c2a4ed454e8f7615d50a02b7be0f18b87926a790 Mon Sep 17 00:00:00 2001 From: Brice Goglin Date: Fri, 8 Jul 2016 09:44:01 +0200 Subject: [PATCH] linux: fix a getmntent() race during load() getmntent() isn't thread safe, it uses a static internal struct mntent. Two concurrent hwloc_topology_load() would break if they call getmntent() (for finding the cgroup directory) at the same time. Use getmntent_r() instead. However we have to specify a buffer for storing mntent strings. And getmntent_r() doesn't actually report an error if the buffer is too small, it silently truncates output strings, so we can't dynamically realloc that buffer. The getmntent() that we used before this commit was internally limited to 4kB. And Linux actually limits mount options to 3 pages (during mount, not when reading /proc/mounts). So use 4 pages to be above both. Thanks to Corentin Rossignon for reporting the issue (using gcc -fsanitize=thread) Fixes #194 Refs #142 (cherry picked from commit 2337361aa9700c31704bf529b7fa01a4abd9df38) --- src/topology-linux.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/topology-linux.c b/src/topology-linux.c index a8a7951c7a..2aae2da5b0 100644 --- a/src/topology-linux.c +++ b/src/topology-linux.c @@ -1713,9 +1713,11 @@ static void hwloc_find_linux_cpuset_mntpnt(char **cgroup_mntpnt, char **cpuset_mntpnt, const char *root_path) { char *mount_path; - struct mntent *mntent; + struct mntent mntent; FILE *fd; int err; + size_t bufsize; + char *buf; *cgroup_mntpnt = NULL; *cpuset_mntpnt = NULL; @@ -1733,14 +1735,25 @@ hwloc_find_linux_cpuset_mntpnt(char **cgroup_mntpnt, char **cpuset_mntpnt, const if (!fd) return; - while ((mntent = getmntent(fd)) != NULL) { - if (!strcmp(mntent->mnt_type, "cpuset")) { - hwloc_debug("Found cpuset mount point on %s\n", mntent->mnt_dir); - *cpuset_mntpnt = strdup(mntent->mnt_dir); + /* getmntent_r() doesn't actually report an error when the buffer + * is too small. It just silently truncates things. So we can't + * dynamically resize things. + * + * Linux limits mount type, string, and options to one page each. + * getmntent() limits the line size to 4kB. + * so use 4*pagesize to be far above both. + */ + bufsize = hwloc_getpagesize()*4; + buf = malloc(bufsize); + + while (getmntent_r(fd, &mntent, buf, bufsize)) { + if (!strcmp(mntent.mnt_type, "cpuset")) { + hwloc_debug("Found cpuset mount point on %s\n", mntent.mnt_dir); + *cpuset_mntpnt = strdup(mntent.mnt_dir); break; - } else if (!strcmp(mntent->mnt_type, "cgroup")) { + } else if (!strcmp(mntent.mnt_type, "cgroup")) { /* found a cgroup mntpnt */ - char *opt, *opts = mntent->mnt_opts; + char *opt, *opts = mntent.mnt_opts; int cpuset_opt = 0; int noprefix_opt = 0; /* look at options */ @@ -1753,16 +1766,17 @@ hwloc_find_linux_cpuset_mntpnt(char **cgroup_mntpnt, char **cpuset_mntpnt, const if (!cpuset_opt) continue; if (noprefix_opt) { - hwloc_debug("Found cgroup emulating a cpuset mount point on %s\n", mntent->mnt_dir); - *cpuset_mntpnt = strdup(mntent->mnt_dir); + hwloc_debug("Found cgroup emulating a cpuset mount point on %s\n", mntent.mnt_dir); + *cpuset_mntpnt = strdup(mntent.mnt_dir); } else { - hwloc_debug("Found cgroup/cpuset mount point on %s\n", mntent->mnt_dir); - *cgroup_mntpnt = strdup(mntent->mnt_dir); + hwloc_debug("Found cgroup/cpuset mount point on %s\n", mntent.mnt_dir); + *cgroup_mntpnt = strdup(mntent.mnt_dir); } break; } } + free(buf); endmntent(fd); }