From: "Ambrož Bizjak" Subject: [PATCH] msvcrt: Make strlwr/strupr write to the string only if necessary. Message-Id: Date: Wed, 27 Nov 2019 19:56:11 +0100 Hi, This patch fixes a crash in IL-2 Sturmovik: Battle of Stanlingrad (with some other tweaks the game runs well). Without this, the game crashes when starting a mission: Unhandled exception: page fault on write access to 0x1408b81bc in 64-bit code (0x00007f783f3ad8e4). ... Backtrace: =>0 0x00007f783f3ad8e4 MSVCRT__strlwr_s_l+0x64() in ucrtbase (0x000000004a77ccf0) 1 0x00007f783f3ad96f MSVCRT__strlwr+0x1e(str="idle") [Z:\build\wine-4.16\dlls\ucrtbase\..\msvcrt\string.c:111] in ucrtbase (0x000000004a77cd30) 2 0x000000014045a782 EntryPoint+0xffbf1fd1() in il-2 (0x000000004a77cee0) 0x00007f783f3ad8e4 MSVCRT__strlwr_s_l+0x64 in ucrtbase: movb %al,0xffffffffffffffff(%rbx) Based on this backtrace, my hypothesis was that the game is calling strlwr on a read-only string that is already lower-case. I have then changed strlwr to not assign characters to the string when the value is unchanged, which fixed the crash. To me this is sufficient evidence to confirm the hypothesis. commit f0a5bfd164afe02b65c49ecb4a94f6fe4495c27b Author: Ambroz Bizjak Date: Wed Nov 27 18:48:24 2019 +0100 msvcrt: Make strlwr/strupr write to the string only if necessary. This fixes a crash in IL-2 Sturmovik: Battle of Stalingrad. It seems that the game is calling strlwr on a read-only string that is already lower-case, which doesn't crash in Windows because strlwr happens not to write to the string (such behavior is not documented). Signed-off-by: Ambroz Bizjak diff --git a/dlls/msvcrt/string.c b/dlls/msvcrt/string.c index fa5f7022c1..155082f5c2 100644 --- a/dlls/msvcrt/string.c +++ b/dlls/msvcrt/string.c @@ -79,7 +79,10 @@ int CDECL MSVCRT__strlwr_s_l(char *str, MSVCRT_size_t len, MSVCRT__locale_t loca while (*str) { - *str = MSVCRT__tolower_l((unsigned char)*str, locale); + char new_ch = (char)MSVCRT__tolower_l((unsigned char)*str, locale); + if (*str != new_ch) { + *str = new_ch; + } str++; } @@ -157,7 +160,10 @@ int CDECL MSVCRT__strupr_s_l(char *str, MSVCRT_size_t len, MSVCRT__locale_t loca { while (*str) { - *str = MSVCRT__toupper_l((unsigned char)*str, locale); + char new_ch = (char)MSVCRT__toupper_l((unsigned char)*str, locale); + if (*str != new_ch) { + *str = new_ch; + } str++; } } diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c index d14c7f009c..ace7850322 100644 --- a/dlls/msvcrt/tests/string.c +++ b/dlls/msvcrt/tests/string.c @@ -2411,6 +2411,11 @@ static void test__strlwr_s(void) ok(!memcmp(buffer, "gorrister\0ELLEN", sizeof("gorrister\0ELLEN")), "Expected the output buffer to be \"gorrister\\0ELLEN\", got \"%s\"\n", buffer); + + /* Test that the function does not write to the string if it would be + unchanged, by passing a string literal that is already lower-case. */ + ret = p_strlwr_s((char *)"already_lowercase", sizeof("already_lowercase")); + ok(ret == 0, "Expected _strlwr_s to return 0, got %d\n", ret); } static void test_wcsncat_s(void) @@ -3598,6 +3603,7 @@ static void test__strupr(void) const char str[] = "123"; char str2[4]; char *mem, *p; + const char *const_p; DWORD prot; mem = VirtualAlloc(NULL, sizeof(str), MEM_COMMIT, PAGE_READWRITE); @@ -3632,6 +3638,12 @@ static void test__strupr(void) ok(!strcmp(mem, "123"), "mem = %s\n", mem); } + /* Test that the function does not write to the string if it would be + unchanged, by passing a string literal that is already upper-case. */ + const_p = "ALREADY_UPPERCASE"; + p = _strupr((char *)const_p); + ok(p == const_p, "_strupr returned %p\n", p); + setlocale(LC_ALL, "C"); VirtualFree(mem, sizeof(str), MEM_RELEASE); }