| Summary: | Unsafe modification to process environment | ||
|---|---|---|---|
| Product: | dbus | Reporter: | Remi Denis-Courmont <courmisch> |
| Component: | core | Assignee: | Havoc Pennington <hp> |
| Status: | RESOLVED FIXED | QA Contact: | John (J5) Palmieri <johnp> |
| Severity: | normal | ||
| Priority: | medium | CC: | chengwei.yang.cn, mirsal.ennaime |
| Version: | 1.2.x | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | threads | ||
| i915 platform: | i915 features: | ||
|
Description
Remi Denis-Courmont
2010-04-11 09:11:09 UTC
The reason for this is that those two env variables apply only to the current process, and should not be exported down to any child processes. I think it's bad practice in general to keep the result of getenv() without making a copy, since that would preclude anyone ever modifying the environment. There's still a theoretical concurrency issue but perhaps starting a thread to init dbus while reading the environment simultaneously in another thread is just something to avoid. i.e. read the environment before, or after, dbus setup, and make a copy of the getenv'd value. Another approach would be to do your getenv() prior to starting threads and save the values. Another workaround would be to putenv() these two vars at the start of main() before other stuff runs, so the putnev in dbus would be a no-op There isn't really a great solution that I see, since the libc interface just isn't threadsafe. But there are tons of workarounds. I wonder if it wouldn't be possible to store some flags in a static variable. libdbus could then remember it has already 'read' those two environment variables. If init_connections_unlocked() is executed once more, it can then pretend the variables don't exist. That avoids using putenv(). Agreeably, this won't propagate to child processes, won't survive dlclose() (which libdbus does not really support anyway). It also won't work if there is another dbus implementation running in the same process, but that seems unlikely. Unfortunately, they're usage scenarios where you simply can't ensure that libdbus is initialized before pthread_create(). The only goal here is to keep those vars away from child processes, though. It's already OK within the same process (or easy to make OK, anyhow). I'm not sure these vars are really used much, so maybe they could just never be set by the parent, or cleared here, but then again, I'm not sure this thread safety issue really happens much either ... it's all obscure situations. Since multithreaded libdbus use is relatively rare I'd lean toward keeping things correct in single-threaded apps, and let threaded apps adapt or come up with a fix. (In reply to comment #2) > Unfortunately, they're usage scenarios where you simply can't ensure that > libdbus is initialized before pthread_create(). Then you can't safely use libdbus. Try GDBus? (In reply to comment #3) > then again, I'm not sure this > thread safety issue really happens much either ... it's all obscure > situations Story of libdbus' life... (In reply to comment #1) > The reason for this is that those two env variables apply only to the > current process, and should not be exported down to any child processes. True. On the other hand, we are in fact clearing the wrong environment variables anyway (see Bug #68308), so it can't have mattered that much. I'd be tempted to just delete this. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.