|
Message-ID: <20150305013457.GA20317@wilbur.25thandClement.com> Date: Wed, 4 Mar 2015 17:34:57 -0800 From: William Ahern <william@...handClement.com> To: musl@...ts.openwall.com Subject: Re: getenv_r On Thu, Mar 05, 2015 at 01:41:33AM +0100, Szabolcs Nagy wrote: > * William Ahern <william@...handClement.com> [2015-03-04 15:09:20 -0800]: > > I noticed that getenv is not thread-safe. Would there be any interest in > > accepting a patch to implement getenv_r (a NetBSD function) and internal > > locking? Other than leaving getenv, setenv, and putenv unsafe in threaded > > environments, the only other alternative is the ugliness that glibc, > > Solaris, and some others implement, which is basically to leak environ > > memory. > > getenv is thread-safe if the environ is not modified > > in a multi-threaded application environ modification > is unsafe and problematic even if you do the locks: > different threads may want different value for an env > var and when you read an env var you cannot know if > it is up to date when you want to use it. That criticism applies to almost any software, no matter the interface, with or without locks. Locks don't solve all TOCTTOU bugs, either. Anyhow, any use of setenv is unsafe in a multi-threaded environment, even for different variables. In musl the __environ array is invalidated by a setenv call that needs to grow it. The system(3) implementation in musl passes __environ to posix_spawn. > does netbsd use getenv_r somewhere to solve some issue? There are a few uses in NetBSD, such as in librump, but AFAICT getenv_r isn't widely used. It just seems a much nicer interface than the contortions other libc libraries go through. One obvious use for a thread-safe setenv and getenv is when trying to generate a time_t timestamp from a UTC struct tm. timegm is not standard. The glibc manual page suggests to instead set the TZ environment variable to UTC, call tzset, call mktime, restore TZ, and call tzset again. You can't use that technique portably from multiple threads unless all code that might call setenv for any reason synchronizes on the same lock you use to implement timegm (including any locale code that needs to read LC_ values). Whereas the only code _setting_ TZ is likely to be the timegm code. But really the issue is most intractable in my position, where I'm trying to implement a bindings module. In Lua especially, where the Lua VM has no global state, it's simple to run Lua scripts in a multi-threaded environment and have them mostly just work without issue. But those scripts are exceedingly unlikely to have been concerned with thread-safety, and with setenv being unsafe to use. And they can't be expected to use synchronization primatives because module A might have no relationship to module B. I can't paper-over all thread issues, but I still think it worthwhile to make it as safe as possible. No amount of finger pointing would ever fix the problem. Granted, at the end of the day this may be none of musl's concern. Adding getenv_r certainly won't solve all issues. That would require other measures. But it does seem like a worthwhile QoI issue to address. I mean, presumably the system implementation uses posix_spawn rather out of concern for QoI. musl could have punted and simply disclaimed any support for threaded environments.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.