diff options
| author | Florian Weimer <fweimer@redhat.com> | 2019-05-21 10:19:46 +0200 |
|---|---|---|
| committer | Florian Weimer <fweimer@redhat.com> | 2019-05-21 12:04:55 +0200 |
| commit | c9c15ac3162d10a999bf71418fd710bf5676175e (patch) | |
| tree | 2264f9e56c7bad8288111a10ac2ff1aad8a317b3 | |
| parent | 7e740ab2e7be7d83b75513aa406e0b10875f7f9c (diff) | |
| download | glibc-c9c15ac3162d10a999bf71418fd710bf5676175e.tar.gz glibc-c9c15ac3162d10a999bf71418fd710bf5676175e.tar.xz glibc-c9c15ac3162d10a999bf71418fd710bf5676175e.zip | |
wcsmbs: Fix data race in __wcsmbs_clone_conv [BZ #24584]
This also adds an overflow check and documents the synchronization requirement in <gconv.h>.
| -rw-r--r-- | ChangeLog | 9 | ||||
| -rw-r--r-- | iconv/gconv.h | 2 | ||||
| -rw-r--r-- | wcsmbs/wcsmbsload.c | 22 |
3 files changed, 29 insertions, 4 deletions
diff --git a/ChangeLog b/ChangeLog index 1dcb620e9a..35e6f7c322 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2019-05-21 Florian Weimer <fweimer@redhat.com> + [BZ #24584] + * wcsmbs/wcsmbsload.c (__wcsmbs_clone_conv): Acquire __gconv_lock + before updating __counter field and release it afterwards. Add + overflow check. + * iconv/gconv.h (struct __gconv_step): Mention synchronization + requirement for __counter member. + +2019-05-21 Florian Weimer <fweimer@redhat.com> + [BZ #24583] * wcsmbs/wcsmbsload.h (__wcsmbs_close_conv): Declare. * wcsmbs/wcsmbsload.c (__wcsmbs_close_conv): Define. diff --git a/iconv/gconv.h b/iconv/gconv.h index 5ad26c06ac..7ce79bcbf6 100644 --- a/iconv/gconv.h +++ b/iconv/gconv.h @@ -86,6 +86,8 @@ struct __gconv_step struct __gconv_loaded_object *__shlib_handle; const char *__modname; + /* For internal use by glibc. (Accesses to this member must occur + when the internal __gconv_lock mutex is acquired). */ int __counter; char *__from_name; diff --git a/wcsmbs/wcsmbsload.c b/wcsmbs/wcsmbsload.c index 10e1a4f4f5..840d4abc44 100644 --- a/wcsmbs/wcsmbsload.c +++ b/wcsmbs/wcsmbsload.c @@ -20,6 +20,7 @@ #include <langinfo.h> #include <limits.h> #include <stdlib.h> +#include <stdio.h> #include <string.h> #include <locale/localeinfo.h> @@ -223,12 +224,25 @@ __wcsmbs_clone_conv (struct gconv_fcts *copy) /* Copy the data. */ *copy = *orig; - /* Now increment the usage counters. - Note: This assumes copy->*_nsteps == 1. */ + /* Now increment the usage counters. Note: This assumes + copy->*_nsteps == 1. The current locale holds a reference, so it + is still there after acquiring the lock. */ + + __libc_lock_lock (__gconv_lock); + + bool overflow = false; if (copy->towc->__shlib_handle != NULL) - ++copy->towc->__counter; + overflow |= __builtin_add_overflow (copy->towc->__counter, 1, + ©->towc->__counter); if (copy->tomb->__shlib_handle != NULL) - ++copy->tomb->__counter; + overflow |= __builtin_add_overflow (copy->tomb->__counter, 1, + ©->tomb->__counter); + + __libc_lock_unlock (__gconv_lock); + + if (overflow) + __libc_fatal ("\ +Fatal glibc error: gconv module reference counter overflow\n"); } |