[hatari-devel] Joystick dialog crashes with Ubuntu 24.04

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

User is reporting that opening SDL GUI Joystick dialog crashes Hatari, 
on Lubuntu 24.04:
	https://www.atari-forum.com/viewtopic.php?p=466100

Is anybody here using some Ubuntu 24.04 variant?   And if yes, can you 
reproduce the issue?


I took a quick look at the related code, and it's possible that SDL 
Joystick name query may get given invalid struct address if SDL reports 
error or no joysticks, or user has invalid joystick mapping in his 
config file.

Attached patch will hopefully handle such issues.  Any comments on it?


	- Eero

From 0674da3b51a031cbcff0877fe5be1ddfdc68f019 Mon Sep 17 00:00:00 2001
From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date: Fri, 5 Jul 2024 16:51:29 +0300
Subject: [PATCH] Prevent invalid joystick mapping causing invalid array access

---
 src/configuration.c       | 13 +++++++++++++
 src/gui-sdl/dlgJoystick.c |  2 +-
 src/joy.c                 | 16 ++++++++++++----
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/configuration.c b/src/configuration.c
index 7a431f6c..1c5f9570 100644
--- a/src/configuration.c
+++ b/src/configuration.c
@@ -957,6 +957,19 @@ void Configuration_Apply(bool bReset)
 	M68000_CheckCpuSettings();
 //fprintf (stderr,"M68000_CheckCpuSettings conf 2\n" );
 
+	/* Disable invalid joystick mappings */
+	for (i = 0; i < JOYSTICK_COUNT; i++)
+	{
+		int joyid = ConfigureParams.Joysticks.Joy[i].nJoyId;
+		if (joyid < 0 || joyid >= JOYSTICK_COUNT)
+		{
+			/* otherwise it may result in invalid array access */
+			Log_Printf(LOG_WARN, "Selected real Joystick %d unavailable, disabling ST joystick %d\n", joyid, i);
+			ConfigureParams.Joysticks.Joy[i].nJoystickMode = JOYSTICK_DISABLED;
+			ConfigureParams.Joysticks.Joy[i].nJoyId = 0;
+		}
+	}
+
 	/* Clean file and directory names */
 	File_MakeAbsoluteName(ConfigureParams.Rom.szTosImageFileName);
 	if (strlen(ConfigureParams.Rom.szCartridgeImageFileName) > 0)
diff --git a/src/gui-sdl/dlgJoystick.c b/src/gui-sdl/dlgJoystick.c
index 791d4367..a33584f8 100644
--- a/src/gui-sdl/dlgJoystick.c
+++ b/src/gui-sdl/dlgJoystick.c
@@ -285,7 +285,7 @@ static void DlgJoystick_ReadValuesFromConf(int nActJoy)
 	int i;
 
 	/* Check if joystick ID is available */
-	if (SDL_NumJoysticks() == 0)
+	if (SDL_NumJoysticks() <= 0)
 	{
 		strcpy(sSdlStickName, "0: (none available)");
 	}
diff --git a/src/joy.c b/src/joy.c
index d3f75c41..09de97a7 100644
--- a/src/joy.c
+++ b/src/joy.c
@@ -78,14 +78,17 @@ const char *Joy_GetName(int id)
 }
 
 /**
- * Return maximum available real joystick ID
+ * Return maximum available real joystick ID, or
+ * zero on error or no joystick (to avoid invalid array accesses)
  */
 int Joy_GetMaxId(void)
 {
 	int count = SDL_NumJoysticks();
 	if (count > JOYSTICK_COUNT)
 		count = JOYSTICK_COUNT;
-	return count - 1;
+	if (count > 0)
+		return count - 1;
+	return 0;
 }
 
 /**
@@ -93,11 +96,13 @@ int Joy_GetMaxId(void)
  */
 bool Joy_ValidateJoyId(int i)
 {
+	int joyid = ConfigureParams.Joysticks.Joy[i].nJoyId;
+
 	/* Unavailable joystick ID -> disable it if necessary */
 	if (ConfigureParams.Joysticks.Joy[i].nJoystickMode == JOYSTICK_REALSTICK &&
-	    !bJoystickWorking[ConfigureParams.Joysticks.Joy[i].nJoyId])
+	    !bJoystickWorking[joyid])
 	{
-		Log_Printf(LOG_WARN, "Selected real Joystick %d unavailable, disabling ST joystick %d\n", ConfigureParams.Joysticks.Joy[i].nJoyId, i);
+		Log_Printf(LOG_WARN, "Selected real Joystick %d unavailable, disabling ST joystick %d\n", joyid, i);
 		ConfigureParams.Joysticks.Joy[i].nJoystickMode = JOYSTICK_DISABLED;
 		ConfigureParams.Joysticks.Joy[i].nJoyId = 0;
 		return false;
@@ -187,7 +192,10 @@ void Joy_UnInit(void)
 		if (bJoystickWorking[i] == true)
 		{
 			SDL_JoystickClose(sdlJoystick[i]);
+			bJoystickWorking[i] = false;
 		}
+		sdlJoystickMapping[i] = NULL;
+		sdlJoystick[i] = NULL;
 	}
 }
 
-- 
2.39.2