duplocale: protect use of global locale (bug 23970)

Message ID mvm5xxzsezz.fsf@suse.de
State Committed
Commit 513331b788a3fa633f1d0417d43915e16a0c88f0
Headers
Series duplocale: protect use of global locale (bug 23970) |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

Andreas Schwab March 6, 2024, 12:48 p.m. UTC
  Protect the global locale from being modified while we compute the size of
the locale category names.  That allows the use of the global locale in a
single thread, while all other threads use the thread safe locale
functions.
---
 locale/duplocale.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
  

Comments

Carlos O'Donell March 7, 2024, 8:37 p.m. UTC | #1
On 3/6/24 07:48, Andreas Schwab wrote:
> Protect the global locale from being modified while we compute the size of
> the locale category names.  That allows the use of the global locale in a
> single thread, while all other threads use the thread safe locale
> functions.

Agreed. Good catch, we should make this work.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  locale/duplocale.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/locale/duplocale.c b/locale/duplocale.c
> index a755ac5c36..bad476700f 100644
> --- a/locale/duplocale.c
> +++ b/locale/duplocale.c
> @@ -43,6 +43,11 @@ __duplocale (locale_t dataset)
>    int cnt;
>    size_t names_len = 0;
>  
> +  /* If dataset points to _nl_global_locale, we need to prevent other
> +     threads from modifying it.  We also modify global data below (the
> +     usage counts).  */
> +  __libc_rwlock_wrlock (__libc_setlocale_lock);
> +
>    /* Calculate the total space we need to store all the names.  */
>    for (cnt = 0; cnt < __LC_LAST; ++cnt)
>      if (cnt != LC_ALL && dataset->__names[cnt] != _nl_C_name)
> @@ -55,9 +60,6 @@ __duplocale (locale_t dataset)
>      {
>        char *namep = (char *) (result + 1);
>  
> -      /* We modify global data (the usage counts).  */
> -      __libc_rwlock_wrlock (__libc_setlocale_lock);
> -
>        for (cnt = 0; cnt < __LC_LAST; ++cnt)
>  	if (cnt != LC_ALL)
>  	  {
> @@ -78,11 +80,11 @@ __duplocale (locale_t dataset)
>        result->__ctype_b = dataset->__ctype_b;
>        result->__ctype_tolower = dataset->__ctype_tolower;
>        result->__ctype_toupper = dataset->__ctype_toupper;
> -
> -      /* It's done.  */
> -      __libc_rwlock_unlock (__libc_setlocale_lock);
>      }
>  
> +  /* It's done.  */
> +  __libc_rwlock_unlock (__libc_setlocale_lock);
> +
>    return result;
>  }
>  weak_alias (__duplocale, duplocale)
  

Patch

diff --git a/locale/duplocale.c b/locale/duplocale.c
index a755ac5c36..bad476700f 100644
--- a/locale/duplocale.c
+++ b/locale/duplocale.c
@@ -43,6 +43,11 @@  __duplocale (locale_t dataset)
   int cnt;
   size_t names_len = 0;
 
+  /* If dataset points to _nl_global_locale, we need to prevent other
+     threads from modifying it.  We also modify global data below (the
+     usage counts).  */
+  __libc_rwlock_wrlock (__libc_setlocale_lock);
+
   /* Calculate the total space we need to store all the names.  */
   for (cnt = 0; cnt < __LC_LAST; ++cnt)
     if (cnt != LC_ALL && dataset->__names[cnt] != _nl_C_name)
@@ -55,9 +60,6 @@  __duplocale (locale_t dataset)
     {
       char *namep = (char *) (result + 1);
 
-      /* We modify global data (the usage counts).  */
-      __libc_rwlock_wrlock (__libc_setlocale_lock);
-
       for (cnt = 0; cnt < __LC_LAST; ++cnt)
 	if (cnt != LC_ALL)
 	  {
@@ -78,11 +80,11 @@  __duplocale (locale_t dataset)
       result->__ctype_b = dataset->__ctype_b;
       result->__ctype_tolower = dataset->__ctype_tolower;
       result->__ctype_toupper = dataset->__ctype_toupper;
-
-      /* It's done.  */
-      __libc_rwlock_unlock (__libc_setlocale_lock);
     }
 
+  /* It's done.  */
+  __libc_rwlock_unlock (__libc_setlocale_lock);
+
   return result;
 }
 weak_alias (__duplocale, duplocale)