Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getmntent() thread-unsafe in hwloc_topology_load() on Linux #194

Closed
corossig opened this issue Jul 6, 2016 · 2 comments
Closed

getmntent() thread-unsafe in hwloc_topology_load() on Linux #194

corossig opened this issue Jul 6, 2016 · 2 comments

Comments

@corossig
Copy link

corossig commented Jul 6, 2016

Hi,
Thread sanitizer report a data race in getmntent which is called at least by hwloc_topology_load.

How to reproduce

Call the sequence "hwloc_topology_init, hwloc_topology_load and hwloc_get_cpubind" in two threads.

Compile the example

with a recent gcc (> 4.8)
gcc -fsanitize=thread getmntent_tsan.c -lhwloc -lpthread

Possible solutions

Use re-entrant version getmntent_r
OR
Put a static mutex around call of hwloc_find_linux_cpuset_mntpnt.

System informations

Linux 2.6.32-504.el6.x86_64 #1 SMP Wed Oct 15 04:27:16 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
Distributor ID: CentOS
Description: CentOS release 6.6 (Final)
Release: 6.6
Codename: Final
Hwloc 1.11.3
gcc 5.3

tsan.txt
getmntent_tsan.c.txt

@bgoglin
Copy link
Contributor

bgoglin commented Jul 6, 2016

I look at using getmntent_r(), thanks.

@bgoglin
Copy link
Contributor

bgoglin commented Jul 6, 2016

By the way, you should use a single topology for both threads whenever possible.

@bgoglin bgoglin changed the title Data race in hwloc_topology_load getmntent() thread-unsafe in hwloc_topology_load() on Linux Jul 7, 2016
@bgoglin bgoglin closed this as completed in 2337361 Jul 8, 2016
bgoglin added a commit that referenced this issue Jul 8, 2016
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 2337361)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants