From d51f84a2e17125e84d4eef0df4fd16c27daf9429 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Fri, 26 May 2023 23:16:45 -0700 Subject: [PATCH] Revert "Fixed double-free during multi-threaded hidapi access" This reverts commit 2b386b6c80b899c5d77cc68856dbab758ae2985f. This isn't the right approach. Even if the string itself isn't double-freed, it can be returned to the application and then freed while the application is trying to use it. This really needs to be in thread-local storage to be completely safe. In SDL we already have a global thread-local error string, so I'm going to make an SDL-specific change to handle the error strings safely. --- src/hidapi/SDL_hidapi.c | 1 - src/hidapi/linux/hid.c | 15 ++------------- src/hidapi/mac/hid.c | 15 ++------------- src/hidapi/windows/hid.c | 22 ++++------------------ 4 files changed, 8 insertions(+), 45 deletions(-) diff --git a/src/hidapi/SDL_hidapi.c b/src/hidapi/SDL_hidapi.c index e32213f2ba..6ca699c642 100644 --- a/src/hidapi/SDL_hidapi.c +++ b/src/hidapi/SDL_hidapi.c @@ -529,7 +529,6 @@ static void HIDAPI_ShutdownDiscovery(void) /* Platform HIDAPI Implementation */ #define HIDAPI_IGNORE_DEVICE(VID, PID) SDL_HIDAPI_ShouldIgnoreDevice(VID, PID) -#define HIDAPI_ATOMIC_SET_POINTER(a, v) SDL_AtomicSetPtr((void **)a, v) struct PLATFORM_hid_device_; typedef struct PLATFORM_hid_device_ PLATFORM_hid_device; diff --git a/src/hidapi/linux/hid.c b/src/hidapi/linux/hid.c index 815379a4a0..9498a85569 100644 --- a/src/hidapi/linux/hid.c +++ b/src/hidapi/linux/hid.c @@ -130,19 +130,8 @@ static wchar_t *utf8_to_wchar_t(const char *utf8) * Use register_error_str(NULL) to free the error message completely. */ static void register_error_str(wchar_t **error_str, const char *msg) { - wchar_t *old_string; -#ifdef HIDAPI_ATOMIC_SET_POINTER - old_string = HIDAPI_ATOMIC_SET_POINTER(error_str, NULL); -#else - old_string = *error_str; *error_str = NULL; -#endif - if (old_string) { - free(old_string); - } - - if (msg) { - *error_str = utf8_to_wchar_t(msg); - } + free(*error_str); + *error_str = utf8_to_wchar_t(msg); } /* Semilar to register_error_str, but allows passing a format string with va_list args into this function. */ diff --git a/src/hidapi/mac/hid.c b/src/hidapi/mac/hid.c index 1578cfe311..9bc4d52cfc 100644 --- a/src/hidapi/mac/hid.c +++ b/src/hidapi/mac/hid.c @@ -243,19 +243,8 @@ static wchar_t *utf8_to_wchar_t(const char *utf8) * Use register_error_str(NULL) to free the error message completely. */ static void register_error_str(wchar_t **error_str, const char *msg) { - wchar_t *old_string; -#ifdef HIDAPI_ATOMIC_SET_POINTER - old_string = HIDAPI_ATOMIC_SET_POINTER(error_str, NULL); -#else - old_string = *error_str; *error_str = NULL; -#endif - if (old_string) { - free(old_string); - } - - if (msg) { - *error_str = utf8_to_wchar_t(msg); - } + free(*error_str); + *error_str = utf8_to_wchar_t(msg); } /* Similar to register_error_str, but allows passing a format string with va_list args into this function. */ diff --git a/src/hidapi/windows/hid.c b/src/hidapi/windows/hid.c index edfd87b47b..b7389d7120 100644 --- a/src/hidapi/windows/hid.c +++ b/src/hidapi/windows/hid.c @@ -255,15 +255,8 @@ static void free_hid_device(hid_device *dev) static void register_winapi_error_to_buffer(wchar_t **error_buffer, const WCHAR *op) { - wchar_t *old_string; -#ifdef HIDAPI_ATOMIC_SET_POINTER - old_string = HIDAPI_ATOMIC_SET_POINTER(error_buffer, NULL); -#else - old_string = *error_buffer; *error_buffer = NULL; -#endif - if (old_string) { - free(old_string); - } + free(*error_buffer); + *error_buffer = NULL; /* Only clear out error messages if NULL is passed into op */ if (!op) { @@ -327,15 +320,8 @@ static void register_winapi_error_to_buffer(wchar_t **error_buffer, const WCHAR static void register_string_error_to_buffer(wchar_t **error_buffer, const WCHAR *string_error) { - wchar_t *old_string; -#ifdef HIDAPI_ATOMIC_SET_POINTER - old_string = HIDAPI_ATOMIC_SET_POINTER(error_buffer, NULL); -#else - old_string = *error_buffer; *error_buffer = NULL; -#endif - if (old_string) { - free(old_string); - } + free(*error_buffer); + *error_buffer = NULL; if (string_error) { *error_buffer = _wcsdup(string_error);