Summary: | getaddrinfo is not thread safe against concurrent setenv | ||
---|---|---|---|
Product: | glibc | Reporter: | Jan David Mol <mol> |
Component: | libc | Assignee: | Ulrich Drepper <drepper.fsp> |
Status: | RESOLVED DUPLICATE | ||
Severity: | normal | CC: | bugdal, carlos, fweimer, neleai |
Priority: | P2 | Flags: | fweimer:
security-
|
Version: | 2.14 | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: |
Description
Jan David Mol
2011-10-07 08:26:00 UTC
Not a bug, as far as I can tell. 2.9.1 Thread-Safety Since multi-threaded applications are not allowed to use the environ variable to access or modify any environment variable while any other thread is concurrently modifying any environment variable, any function dependent on any environment variable is not thread-safe if another thread is modifying the environment; see XSH exec. And the cross-referenced text (from exec): Conforming multi-threaded applications shall not use the environ variable to access or modify any environment variable while any other thread is concurrently modifying any environment variable. A call to any function dependent on any environment variable shall be considered a use of the environ variable to access that environment variable. The only way the issue you reported *may* be a bug is in that getaddrinfo is not documented by the standard to use any environment variables; this is a behavior specific to most implementations including glibc. I'm not sure how cases like this should be treated, but I think as long as the implementation documents the use of the environment it's probably technically okay.. As soon as multiple threads are in use a program must not modify the environment and therefore calls to getenv are safe. According to section 2.9.1 you quote, a program is still thread safe if it writes environment variables, and thus allows setenv(), it just can't read them at the same time. Thus the issue arises that getaddrinfo is not thread-safe under some circumstances, i.e. when there is no concurrent use of setenv() and friends. This violates the POSIX requirements for getaddrinfo: The freeaddrinfo() and getaddrinfo() functions shall be thread-safe. I don't see any exceptions being noted or any access to environment variables being mentioned? @Ulrich: where does it state that concurrent threads cannot modify the environment at all? Section 2.9.1 seems to directly contradict that statement. In fact, the glibc documentation does not provide any hint towards the fact that getaddrinfo will clash with setenv, nor does it forbid the use of setenv in multithreaded programs as you seem to imply. 2.9.1 states very explicitly, "any function dependent on any environment variable is not thread-safe if another thread is modifying the environment". The only potential wiggle-room to claim that glibc is non-conformant is that getaddrinfo is not documented (by the standard or by the implementation) as depending on the environment. With that said, I agree that Ulrich Drepper is wrong to claim that modifying the environment in a multi-threaded program is not allowed. It can be done safely in many situations, such as when all but one thread is purely computational or only invoking only async-signal-safe functions. It should also be perfectly safe if you modify extern char **environ; (or the array it points to) directly in only atomic ways and don't clobber any environment memory that other threads could still be referencing. The reason I report this is because I saw my application crash repeatedly in a validly used getaddrinfo, because another thread invalidated environ through setenv. I've since replaced the setenv call, but what worries me is that nothing in the documentation of glibc or POSIX could have indicated for me that this could happen. Although I personally would consider a function only thread safe if no valid use can cause undefined behaviour due to multithreaded use, I do recognise that in this case practicality is important to consider. After all, dropping support for the environment variables used in getaddrinfo and other functions, or wrapping getenv with the same lock that setenv already has, might not be desirable either. Since you are the devs and not me, it is up to you, but I kindly request that you reopen this bug as a documentation one at least. The setenv man page family could carry a warning indicating that it makes the behaviour of concurrently running thread-safe (glibc) functions undefined? I think a reasonable solution that would definitely be conformant would be for getenv and functions documented *by the standard* as depending on the environment to remain as-is, and not perform any locking, but for getaddrinfo and any other functions that use the environment but aren't documented by the standard as doing so to obtain the environment lock before using getenv. Adding locking to getaddrinfo would be a tiny cost anyway in comparison to all the syscalls involved in performing a DNS lookup.. Reopened; this ticket cannot be regarded as 'invalid', nor 'resolved', because, as mentioned earlier, POSIX and glibc docs do allow setenv and getaddrinfo to run concurrently. Rich Felker's idea of wrapping all getenv calls inside glibc is good, assuming the number of functions needing access to that global lock is limited. Getenv itself needs only very weak (in fact, insanely weak) guarantees by POSIX, so indeed that doesn't need change. I've already explained that multi-threaded programs are not allowed to modify the environment. Stop reopening and educate yourself. Please educate yourself: http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_09_01 http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html (In reply to comment #9) > Please educate yourself: Just go away, you really know nothing. Guess who of the two of us is in the POSIX committee? I'm tired of childish insults as a substitute for reading simple text. Changing status to WONTFIX. *** Bug 260998 has been marked as a duplicate of this bug. *** Seen from the domain http://volichat.com Page where seen: http://volichat.com/adult-chat-rooms Marked for reference. Resolved as fixed @bugzilla. Reopening since this should be fixed. It couldn't be fixed in general as Rich pointed out that user could setup custom environment and deallocate it. Anyway this is duplicate of generic request for threadsafe getenv. That should be fixed in generic way. *** This bug has been marked as a duplicate of bug 15607 *** |