From 2b386b6c80b899c5d77cc68856dbab758ae2985f Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Fri, 26 May 2023 20:28:20 -0700
Subject: [PATCH] Fixed double-free during multi-threaded hidapi access
The error string is not protected by a mutex, and can be set from multiple threads at the same time. Without this change, it can be double-freed. It can still be double-allocated, leading to a memory leak, but at least it won't crash now.
Signed-off-by: Sam Lantinga <slouken@libsdl.org>
---
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, 45 insertions(+), 8 deletions(-)
diff --git a/src/hidapi/SDL_hidapi.c b/src/hidapi/SDL_hidapi.c
index 6ca699c64251..e32213f2baae 100644
--- a/src/hidapi/SDL_hidapi.c
+++ b/src/hidapi/SDL_hidapi.c
@@ -529,6 +529,7 @@ 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 9498a85569aa..815379a4a00a 100644
--- a/src/hidapi/linux/hid.c
+++ b/src/hidapi/linux/hid.c
@@ -130,8 +130,19 @@ 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)
{
- free(*error_str);
- *error_str = utf8_to_wchar_t(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);
+ }
}
/* 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 9bc4d52cfcf6..1578cfe3111e 100644
--- a/src/hidapi/mac/hid.c
+++ b/src/hidapi/mac/hid.c
@@ -243,8 +243,19 @@ 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)
{
- free(*error_str);
- *error_str = utf8_to_wchar_t(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);
+ }
}
/* 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 7c98f5392b0b..bdb3a2c011a5 100644
--- a/src/hidapi/windows/hid.c
+++ b/src/hidapi/windows/hid.c
@@ -255,8 +255,15 @@ static void free_hid_device(hid_device *dev)
static void register_winapi_error_to_buffer(wchar_t **error_buffer, const WCHAR *op)
{
- free(*error_buffer);
- *error_buffer = NULL;
+ 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);
+ }
/* Only clear out error messages if NULL is passed into op */
if (!op) {
@@ -320,8 +327,15 @@ 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)
{
- free(*error_buffer);
- *error_buffer = NULL;
+ 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);
+ }
if (string_error) {
*error_buffer = _wcsdup(string_error);