git: 7eb138a9e536 - main - libc/locale: Fix races between localeconv(3) and setlocale(3)

Mark Johnston markj at FreeBSD.org
Fri Sep 17 15:00:42 UTC 2021


The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=7eb138a9e53636366e615bdf04062fedc044bcea

commit 7eb138a9e53636366e615bdf04062fedc044bcea
Author:     Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-09-17 14:44:23 +0000
Commit:     Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-09-17 14:47:46 +0000

    libc/locale: Fix races between localeconv(3) and setlocale(3)
    
    Each locale embeds a lazily initialized lconv which is populated by
    localeconv(3) and localeconv_l(3).  When setlocale(3) updates the global
    locale, the lconv needs to be (lazily) reinitialized.  To signal this,
    we set flag variables in the locale structure.  There are two problems:
    
    - The flags are set before the locale is fully updated, so a concurrent
      localeconv() call can observe partially initialized locale data.
    - No barriers ensure that localeconv() observes a fully initialized
      locale if a flag is set.
    
    So, move the flag update appropriately, and use acq/rel barriers to
    provide some synchronization.  Note that this is inadequate in the face
    of multiple concurrent calls to setlocale(3), but this is not expected
    to work regardless.
    
    Thanks to Henry Hu <henry.hu.sh at gmail.com> for providing a test case
    demonstrating the race.
    
    PR:             258360
    MFC after:      3 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31899
---
 lib/libc/locale/lmonetary.c  | 4 ++--
 lib/libc/locale/lnumeric.c   | 4 ++--
 lib/libc/locale/localeconv.c | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/libc/locale/lmonetary.c b/lib/libc/locale/lmonetary.c
index 99800ae69922..b8b4e78c060b 100644
--- a/lib/libc/locale/lmonetary.c
+++ b/lib/libc/locale/lmonetary.c
@@ -107,8 +107,6 @@ monetary_load_locale_l(struct xlocale_monetary *loc, int *using_locale,
 		&loc->buffer, "LC_MONETARY",
 		LCMONETARY_SIZE_FULL, LCMONETARY_SIZE_MIN,
 		(const char **)l);
-	if (ret != _LDP_ERROR)
-		*changed = 1;
 	if (ret == _LDP_LOADED) {
 		l->mon_grouping =
 		     __fix_locale_grouping_str(l->mon_grouping);
@@ -146,6 +144,8 @@ monetary_load_locale_l(struct xlocale_monetary *loc, int *using_locale,
 		M_ASSIGN_ICHAR(p_sign_posn);
 		M_ASSIGN_ICHAR(n_sign_posn);
 	}
+	if (ret != _LDP_ERROR)
+		atomic_store_rel_int(changed, 1);
 	return (ret);
 }
 int
diff --git a/lib/libc/locale/lnumeric.c b/lib/libc/locale/lnumeric.c
index 046d1f1817dc..cc1daa3863e3 100644
--- a/lib/libc/locale/lnumeric.c
+++ b/lib/libc/locale/lnumeric.c
@@ -73,8 +73,6 @@ numeric_load_locale(struct xlocale_numeric *loc, int *using_locale, int *changed
 		&loc->buffer, "LC_NUMERIC",
 		LCNUMERIC_SIZE, LCNUMERIC_SIZE,
 		(const char**)l);
-	if (ret != _LDP_ERROR)
-		*changed= 1;
 	if (ret == _LDP_LOADED) {
 		/* Can't be empty according to C99 */
 		if (*l->decimal_point == '\0')
@@ -83,6 +81,8 @@ numeric_load_locale(struct xlocale_numeric *loc, int *using_locale, int *changed
 		l->grouping =
 		    __fix_locale_grouping_str(l->grouping);
 	}
+	if (ret != _LDP_ERROR)
+		atomic_store_rel_int(changed, 1);
 	return (ret);
 }
 
diff --git a/lib/libc/locale/localeconv.c b/lib/libc/locale/localeconv.c
index 641773944e32..130f93c178f4 100644
--- a/lib/libc/locale/localeconv.c
+++ b/lib/libc/locale/localeconv.c
@@ -65,7 +65,7 @@ localeconv_l(locale_t loc)
 	FIX_LOCALE(loc);
     struct lconv *ret = &loc->lconv;
 
-    if (loc->monetary_locale_changed) {
+    if (atomic_load_acq_int(&loc->monetary_locale_changed) != 0) {
 	/* LC_MONETARY part */
         struct lc_monetary_T * mptr; 
 
@@ -94,10 +94,10 @@ localeconv_l(locale_t loc)
 	M_ASSIGN_CHAR(int_n_sep_by_space);
 	M_ASSIGN_CHAR(int_p_sign_posn);
 	M_ASSIGN_CHAR(int_n_sign_posn);
-	loc->monetary_locale_changed = 0;
+	atomic_store_int(&loc->monetary_locale_changed, 0);
     }
 
-    if (loc->numeric_locale_changed) {
+    if (atomic_load_acq_int(&loc->numeric_locale_changed) != 0) {
 	/* LC_NUMERIC part */
         struct lc_numeric_T * nptr; 
 
@@ -107,7 +107,7 @@ localeconv_l(locale_t loc)
 	N_ASSIGN_STR(decimal_point);
 	N_ASSIGN_STR(thousands_sep);
 	N_ASSIGN_STR(grouping);
-	loc->numeric_locale_changed = 0;
+	atomic_store_int(&loc->numeric_locale_changed, 0);
     }
 
     return ret;


More information about the dev-commits-src-main mailing list