On Fri 2016-07-15 14:19:51 +0200, Maik Zumstrull wrote:
> Package: libnss-extrausers
> Version: 0.6-3
>
> The implementation uses three static global pointer variables:
>
> static FILE *groupsfile = NULL;
> static FILE *shadowfile = NULL;
> static FILE *usersfile = NULL;
>
> Since these are used without locks or atomic operations, this is not
> thread-safe, even though NSS functions are supposed to be
> reentrant. This is leading to occasional "double free or corruption"
> crashes in libvirtd for us, specifically in fclose(groupsfile); in
> _nss_extrausers_endgrent.
>
> As a quick fix, I suggest declaring these variables thread-local:
>
> static __thread FILE *groupsfile = NULL;
> static __thread FILE *shadowfile = NULL;
> static __thread FILE *usersfile = NULL;
>
> It's not necessarily the most elegant solution, but it should make the code
> fully reentrant.
Are you sure this wouldn't cause the different thread's understanding of
the underlying file descriptors to get out of sync? declaring them
thread-local wouldn't prevent them sharing the same file descriptor,
right? i think it would just be thread-local buffering.
This seems like a way to cause some even more subtle bug, for example:
thread A causes libnss-extrausers to scan the groups file for some
value, and it reads up to block X, while
thread B causes libnss-extrausers to scan the groups file for some
other value, and it reads up to block Y
then thread A resumes; the underlying file descriptor is advanced to
position Y but the FILE* buffers in thread A have only cached enough
data to represent block X. what happens then?
I could be misunderstanding the libnss-extrausers codebase, though --
feel free to clarify and correct!
--dkg
