[PATCH v2 0/2] MR10785: msvcrt: Only reject negative timestamps with EINVAL in ctime_s
0 needs to be an allowed value, and negative values should not halt the program but just return EINVAL. This behavior has been observed with the following python script: ```python import ctypes ucrt = ctypes.CDLL("ucrtbase.dll") buf = ctypes.create_string_buffer(26) for i in [1, 0, -1]: print(f"_ctime64_s for time {i}:") t = ctypes.c_int64(i) err = ucrt._ctime64_s(buf, ctypes.c_size_t(26), ctypes.byref(t)) if err == 0: print(buf.value.decode()) else: print(f"Error: {err}") for i in [1, 0, -1]: print(f"_ctime32_s for time {i}:") t = ctypes.c_int32(i) err = ucrt._ctime32_s(buf, ctypes.c_size_t(26), ctypes.byref(t)) if err == 0: print(buf.value.decode()) else: print(f"Error: {err}") # _ctime64_s for time 1: # Thu Jan 1 01:00:01 1970 # # _ctime64_s for time 0: # Thu Jan 1 01:00:00 1970 # # _ctime64_s for time -1: # Error: 22 # # _ctime32_s for time 1: # Thu Jan 1 01:00:01 1970 # # _ctime32_s for time 0: # Thu Jan 1 01:00:00 1970 # # _ctime32_s for time -1: # Error: 22 ``` The bug https://bugs.winehq.org/show_bug.cgi?id=57261 is solved by this, because mongodb tries to pretty-print a 0 timestamp, and sets up its own invalid_parameter_handler, which crashes due to NULL arguments. -- v2: msvcrt: Only reject negative timestamps with EINVAL in ctime_s msvcrt/tests: Test that ctime_s succeeds for timestamp 0 https://gitlab.winehq.org/wine/wine/-/merge_requests/10785
From: Hendrik Borchardt <hendrik.borchardt@gmail.com> --- dlls/msvcrt/tests/time.c | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/dlls/msvcrt/tests/time.c b/dlls/msvcrt/tests/time.c index aba911f30a4..c72280cccb2 100644 --- a/dlls/msvcrt/tests/time.c +++ b/dlls/msvcrt/tests/time.c @@ -51,6 +51,8 @@ typedef struct { int refcount; } __lc_time_data; +static errno_t (__cdecl *p_ctime32_s)(char*, size_t, __time32_t*); +static errno_t (__cdecl *p_ctime64_s)(char*, size_t, __time64_t*); static __time32_t (__cdecl *p_mkgmtime32)(struct tm*); static struct tm* (__cdecl *p_gmtime32)(__time32_t*); static struct tm* (__cdecl *p_gmtime)(time_t*); @@ -76,6 +78,8 @@ static void init(void) HMODULE hmod = LoadLibraryA("msvcrt.dll"); p_gmtime32 = (void*)GetProcAddress(hmod, "_gmtime32"); + p_ctime32_s = (void*)GetProcAddress(hmod, "_ctime32_s"); + p_ctime64_s = (void*)GetProcAddress(hmod, "_ctime64_s"); p_gmtime = (void*)GetProcAddress(hmod, "gmtime"); p_gmtime32_s = (void*)GetProcAddress(hmod, "_gmtime32_s"); p_gmtime64 = (void*)GetProcAddress(hmod, "_gmtime64"); @@ -116,6 +120,45 @@ static void test_ctime(void) ret = ctime(&badtime); ok(ret == NULL, "expected ctime to return NULL, got %s\n", ret); } + +static void test_ctime32_s(void) +{ + __time32_t goodtime = 0; + __time32_t badtime = -1; + char out[26]; + int ret; + + if(!p_ctime32_s) { + win_skip("Skipping _ctime32_s tests\n"); + return; + } + + ret = p_ctime32_s(out, 26, &goodtime); + ok(ret == 0, "expected _ctime32_s to return NULL, got %i\n", ret); + + ret = p_ctime32_s(out, 26, &badtime); + ok(ret == EINVAL, "expected _ctime32_s to return EINVAL, got %i\n", ret); +} + +static void test_ctime64_s(void) +{ + __time64_t goodtime = 0; + __time64_t badtime = -1; + char out[26]; + int ret; + + if(!p_ctime64_s) { + win_skip("Skipping _ctime64_s tests\n"); + return; + } + + ret = p_ctime64_s(out, 26, &goodtime); + ok(ret == 0, "expected _ctime64_s to return NULL, got %i\n", ret); + + ret = p_ctime64_s(out, 26, &badtime); + ok(ret == EINVAL, "expected _ctime64_s to return EINVAL, got %i\n", ret); +} + static void test_gmtime(void) { __time32_t valid, gmt; @@ -1038,6 +1081,8 @@ START_TEST(time) test__tzset(); test_strftime(); test_ctime(); + test_ctime32_s(); + test_ctime64_s(); test_gmtime(); test_gmtime64(); test_mktime(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10785
From: Hendrik Borchardt <hendrik.borchardt@gmail.com> 0 needs to be an allowed value, and negative values should not halt the program but just return EINVAL. --- dlls/msvcrt/time.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/msvcrt/time.c b/dlls/msvcrt/time.c index b52bb872647..3e826f04bef 100644 --- a/dlls/msvcrt/time.c +++ b/dlls/msvcrt/time.c @@ -1737,7 +1737,7 @@ errno_t CDECL _ctime64_s(char *res, size_t len, const __time64_t *time) if (!MSVCRT_CHECK_PMT( len >= 26 )) return EINVAL; res[0] = '\0'; if (!MSVCRT_CHECK_PMT( time != NULL )) return EINVAL; - if (!MSVCRT_CHECK_PMT( *time > 0 )) return EINVAL; + if (*time < 0) return EINVAL; ret = _localtime64_s( &t, time ); if (ret) @@ -1768,7 +1768,7 @@ errno_t CDECL _ctime32_s(char *res, size_t len, const __time32_t *time) if (!MSVCRT_CHECK_PMT( len >= 26 )) return EINVAL; res[0] = '\0'; if (!MSVCRT_CHECK_PMT( time != NULL )) return EINVAL; - if (!MSVCRT_CHECK_PMT( *time > 0 )) return EINVAL; + if (*time < 0) return EINVAL; ret = _localtime32_s( &t, time ); if (ret) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10785
Piotr Caban (@piotr) commented about dlls/msvcrt/tests/time.c:
+{ + __time32_t goodtime = 0; + __time32_t badtime = -1; + char out[26]; + int ret; + + if(!p_ctime32_s) { + win_skip("Skipping _ctime32_s tests\n"); + return; + } + + ret = p_ctime32_s(out, 26, &goodtime); + ok(ret == 0, "expected _ctime32_s to return NULL, got %i\n", ret); + + ret = p_ctime32_s(out, 26, &badtime); + ok(ret == EINVAL, "expected _ctime32_s to return EINVAL, got %i\n", ret); The function should also set errno on error:
```suggestion:-1+0 errno = 0; ret = p_ctime32_s(out, 26, &badtime); ok(ret == EINVAL, "expected _ctime32_s to return EINVAL, got %i\n", ret); ok(errno == EINVAL, "errno = %d\n", errno); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10785#note_138369
Thanks for the patches. Tests needs to pass after every commit: please merge or reorder the patches. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10785#note_138370
On Thu Apr 30 12:32:50 2026 +0000, Piotr Caban wrote:
Thanks for the patches. Tests needs to pass after every commit: please merge or reorder the patches. Sure, will do. But maybe this needs to be changed in the wiki?
https://gitlab.winehq.org/wine/wine/-/wikis/Submitting-Patches#patch-guideli...: "If you've written a fix and a lot of new [conformance tests](Conformance-Tests) for an issue, add the tests as a separate commit ~~before~~after the fix" -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10785#note_138372
participants (3)
-
Hendrik Borchardt -
Hendrik Borchardt (＠hborchardt)
-
Piotr Caban (＠piotr)