[Xcb] [PATCH libXau] Avoid heap corruption when calling XauFileName from multiple threads.

[Rami Ylimäki]

On 03/28/2011 09:14 PM, Mark Kettenis wrote:
>> Date: Mon, 28 Mar 2011 10:21:06 -0700
>> From: Alan Coopersmith<alan.coopersmith at oracle.com>
>>
>>> This commit fixes only the heap corruption and sporadic crashes. It's
>>> still possible that XauFileName returns a badly formed filename string
>>> if called from multiple threads. For example, changing contents of
>>> HOME environment variable could make the returned string to be
>>> malformed. However, there shouldn't be crashes.
>> Perhaps we just need to define a new function to do this, that doesn't
>> use a static global variable of any sort, and declare that multi-threaded
>> code needs to use it.
> That does make more sense than adding hacks to limit the potential
> damage.  The current interface simply is inherently thread-unsafe.

I was actually thinking about implementing XauGetFileName as an 
alternative for XauFileName, that would return dynamically allocated 
filename string and require it to be freed by callers. However, the real 
problem is that getenv has to be called at some point to get XAUTHORITY 
or HOME, and that can't be done safely inside a library in any portable way.

The only way to solve the problem in a thread safe manner would be to 
pass copies of XAUTHORITY and HOME to Xlib/Xcb when creating a new 
connection. Alternatively the semantics of connection creation should be 
changed so that environment variables would be completely ignored. Both 
of these options aren't very good from backward compatibility point of view.

We chose to fix the common case and make connection creation safe as 
long as environment is not modified at the same time. We didn't 
implement a new function, because if we offer a new function for 
multithreaded clients, it should be also protected against environment 
changes. That would again require a lot of changes in users of libXau. 
In other words, the fix avoids a real problem and while not perfect, 
it's good enough. 
[http://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good :)]

Could you give your opinion about the environment problem? If a new 
function is created for multithreaded clients, what should be done about 
reading XAUTHORITY and HOME in a safe manner? Alternatively, do you 
think that protecting against environment changes is not that important 
and fixing the allocation problems are enough?

-- Rami