[PATCH 0/1] MR4816: msvcrt: Reference old locinfo for create_locinfo().

[Paul Gofman (＠gofman)]

When threaded locales are not enabled but an app is using setlocale() concurrently, create_locinfo() may use old locale after free. That happens because the reference is held from thread data, but some functions called from within create_locinfo() can get_locinfo() (->update_thread_locale) and that may result in freeing the old locinfo.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4816

[PATCH 1/1] msvcrt: Reference old locinfo for create_locinfo().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/msvcrt/locale.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index f7be2a1e9bc..0a0a5c6bbea 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -2036,6 +2036,7 @@ char* CDECL setlocale(int category, const char* locale)
 {
     thread_data_t *data = msvcrt_get_thread_data();
     pthreadlocinfo locinfo = get_locinfo(), newlocinfo;
+    BOOL need_free;
 
     if(category<LC_MIN || category>LC_MAX)
         return NULL;
@@ -2047,7 +2048,12 @@ char* CDECL setlocale(int category, const char* locale)
         return locinfo->lc_category[category].locale;
     }
 
+    if ((need_free = (locinfo && data->locale_flags & LOCALE_FREE)))
+        grab_locinfo(locinfo);
     newlocinfo = create_locinfo(category, locale, locinfo);
+    if (need_free)
+        free_locinfo(locinfo);
+
     if(!newlocinfo) {
         WARN("%d %s failed\n", category, locale);
         return NULL;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4816

[Piotr Caban (＠piotr)]

It looks for me like something that never worked. Is it a regression?

This is a duplicate of !4654.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4816#note_57057

[Paul Gofman (＠gofman)]

That came to me as a regression in Road to Vostok Demo which was tested to be working on Proton 5.13 but frequently crashing on start starting from Proton 6.3. I didn't try to trace what exact changes could leave to the difference in behaviour, maybe there were some changes which showed this up as regression (possibly unrelated to this place at all) or maybe that was just less likely on older versions.

Sorry, I did not notice that MR before, I will close this one then.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4816#note_57062

[PATCH 0/1] MR4816: msvcrt: Reference old locinfo for create_locinfo(). - closed

[Paul Gofman (＠gofman)]

This merge request was closed by Paul Gofman.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4816