Re: [RFC]setlocale() race condition

[Sharyathi Nagesh <sharyath at in dot ibm dot com>]

Carlos
Excuse me for the late response
	Here is the mail with update as you suggested

========================================================================
Explanation:

During PHP engine development it was observed that a thread calling
strcmp() would crash if another thread was also calling setlocale().
The problem is difficult to reproduce, but the following is a
line-by-line analysis of where the race condition exists.
-----------------------------------------------------------------------
char *
setlocale (int category, const char *locale)
{
  char *locale_path;
  .......
 /* Does user want name of current locale?  */
  if (locale == NULL)
    return (char *) _nl_global_locale.__names[category];

  if (strcmp (locale, _nl_global_locale.__names[category]) == 0)
<== SIGSEGVs while accessing _nl_global_locale

  ..........


if (category == LC_ALL)
    {
  ..........
/* Protect global data.  */
      __libc_rwlock_wrlock (__libc_setlocale_lock);
  ..........
setname (LC_ALL, composite)
<== Frees the global variable _nl_global_locale.__names[category]

..........
 /* Critical section left.  */
      __libc_rwlock_unlock (__libc_setlocale_lock);
} //category == LC_ALL

  ..........


} //setlocale end of function
-----------------------------------------------------------------------
Though setlocale() is not on the POSIX.1 list of async-signal safe
functions as in section 2.4.3 
http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html#tag_02_04

It still needs to be thread safe according to section 2.9.1 in
http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html
========================================================================
Testing:
	There was no regression observed while testing. Testing was done on 
x86_64-linux-gnu box where the application was built as 32 bit.
========================================================================
FSF copyright assignment
	Request for copy right assignment has been forwarded, it is pending on 
approval.
========================================================================
GNU Changelog

2008-06-12	Sharyathi Nagesh	<sharyathi@in.ibm.com>
	* locale/setlocale.c (setlocale): Fix Race condition while 			calling 
strcmp().
========================================================================
Fix:
	This fix protects the global variable with read lock when it is being 
accessed inside the strcmp() function, so preventing any other thread 
from freeing it during the same time.
Thanks
Yeehaw

Index: libc/locale/setlocale.c
===================================================================
--- libc.orig/locale/setlocale.c	2008-03-31 06:07:03.000000000 +0530
+++ libc/locale/setlocale.c	2008-06-12 08:59:45.000000000 +0530
@@ -234,10 +234,14 @@
   if (locale == NULL)
     return (char *) _nl_global_locale.__names[category];
 
+  __libc_rwlock_rdlock (__libc_setlocale_lock);
   if (strcmp (locale, _nl_global_locale.__names[category]) == 0)
+	{
+      __libc_rwlock_unlock (__libc_setlocale_lock);
     /* Changing to the same thing.  */
     return (char *) _nl_global_locale.__names[category];
-
+	}
+      __libc_rwlock_unlock (__libc_setlocale_lock);
   /* We perhaps really have to load some data.  So we determine the
      path in which to look for the data now.  The environment variable
      `LOCPATH' must only be used when the binary has no SUID or SGID