Crash when running with -DHIDAPI=ON

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• fix error checking and initialisation in SDL_hidapi.c (hidapi_error_check.patch, text/plain, 2020-07-05 12:03:26 +0000, 5447 bytes)
• fix error checking and initialisation in SDL_hidapi.c (no log) (hidapi_error_check.patch, text/plain, 2020-07-06 09:01:48 +0000, 5453 bytes)

Reported in version: HG 2.0
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2020-07-03 10:06:09 +0000, Sven-Hendrik Haase wrote:

Applications that init the joystick subsystem crash after I use compile my SDL with -DHIDAPI=ON.

Example running blobby volley (SDL2) with on recent tip with -DHIDAPI=ON:

0 0x0000000000000000 in ()

1 0x00007ffff7e3e555 in LIBUSB_hid_init () at ../src/hidapi/libusb/hid.c:460

2 hid_init () at ../src/hidapi/SDL_hidapi.c:491

3 0x00007ffff7e40435 in hid_init () at ../src/hidapi/SDL_hidapi.c:454

4 0x00007ffff7e3cb99 in HIDAPI_JoystickInit () at ../src/joystick/hidapi/SDL_hidapijoystick.c:582

5 0x00007ffff7e01eb4 in SDL_JoystickInit () at ../src/joystick/SDL_joystick.c:238

6 0x00007ffff7d2a2b5 in SDL_InitSubSystem_REAL (flags=16944) at ../src/SDL.c:234

7 0x0000555555669348 in main ()

I compile my SDL2 like this:

cmake .
-Bbuild
-GNinja
-DCMAKE_INSTALL_PREFIX=/usr
-DSDL_STATIC=OFF
-DSDL_DLOPEN=ON
-DARTS=OFF
-DESD=OFF
-DNAS=OFF
-DALSA=ON
-DHIDAPI=ON
-DPULSEAUDIO_SHARED=ON
-DVIDEO_WAYLAND=ON
-DRPATH=OFF
-DCLOCK_GETTIME=ON
-DJACK_SHARED=ON
ninja -C build

On 2020-07-03 10:07:14 +0000, Sven-Hendrik Haase wrote:

Some more system information:

Arch Linux
cmake 3.17.3
gcc 10.1.0

On 2020-07-03 10:09:50 +0000, Sven-Hendrik Haase wrote:

Our downstream bug report about this: https://bugs.archlinux.org/task/67174

On 2020-07-05 12:03:26 +0000, Mathieu Eyraud wrote:

Created attachment 4402
fix error checking and initialisation in SDL_hidapi.c

SDL dynamically loads libusb but does not check the return value of 'SDL_LoadFunction'.

Also libusb is loaded and initialized several time because 'SDL_hidapi_wasinit' is never set to true.

I made a patch if you want to test:

• check that 'hid_init' is called once and only once,
• check return value of 'hid_init',
• check return value of 'SDL_LoadFunction',
• check return value of 'SDL_malloc',
• add some debug logging.

On 2020-07-05 12:22:48 +0000, Sven-Hendrik Haase wrote:

(In reply to meyraud705 from comment # 3)

Created attachment 4402 [details]
fix error checking and initialisation in SDL_hidapi.c

SDL dynamically loads libusb but does not check the return value of
'SDL_LoadFunction'.

Also libusb is loaded and initialized several time because
'SDL_hidapi_wasinit' is never set to true.

I made a patch if you want to test:

• check that 'hid_init' is called once and only once,
• check return value of 'hid_init',
• check return value of 'SDL_LoadFunction',
• check return value of 'SDL_malloc',
• add some debug logging.

Doesn't compile for me:

FAILED: CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o
/usr/bin/cc -DSDL2_EXPORTS -DSDL_USE_IME -DUSING_GENERATED_CONFIG_H -Iinclude -I../include -Iwayland-generated-protocols -I/usr/include/libdrm -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -I/usr/include/ibus-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/libmount -I/usr/include/blkid -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -idirafter /build/sdl2/src/SDL2-2.0.12/src/video/khronos -I/usr/include/libusb-1.0 -I/build/sdl2/src/SDL2-2.0.12/src/hidapi/hidapi -DHAVE_LINUX_VERSION_H -I/usr/include/libdrm -I/usr/include -D_REENTRANT -msse3 -msse2 -msse -m3dnow -mmmx -Wshadow -fvisibility=hidden -Wdeclaration-after-statement -Werror=declaration-after-statement -fno-strict-aliasing -Wall -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fPIC -MD -MT CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o -MF CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o.d -o CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o -c ../src/hidapi/SDL_hidapi.c
In file included from ../src/hidapi/../SDL_internal.h:45,
from ../src/hidapi/SDL_hidapi.c:31:
../src/hidapi/SDL_hidapi.c: In function ‘hid_init’:
../src/hidapi/../dynapi/SDL_dynapi_overrides.h:35:21: warning: implicit declaration of function ‘SDL_LogWarn_REAL’; did you mean ‘SDL_CondWait_REAL’? [-Wimplicit-function-declaration]
35 | #define SDL_LogWarn SDL_LogWarn_REAL
| ^~~~~~~~~~~~~~~~
../src/hidapi/SDL_hidapi.c:501:13: note: in expansion of macro ‘SDL_LogWarn’
501 | SDL_LogWarn(SDL_LOG_CATEGORY_INPUT, SDL_LIBUSB_DYNAMIC " found but could not load function.");
| ^~~~~~~~~~~
../src/hidapi/SDL_hidapi.c:501:25: error: ‘SDL_LOG_CATEGORY_INPUT’ undeclared (first use in this function)
501 | SDL_LogWarn(SDL_LOG_CATEGORY_INPUT, SDL_LIBUSB_DYNAMIC " found but could not load function.");
| ^~~~~~~~~~~~~~~~~~~~~~

On 2020-07-06 09:01:48 +0000, Mathieu Eyraud wrote:

Created attachment 4403
fix error checking and initialisation in SDL_hidapi.c (no log)

Same patch with problematic line commented out (you will not get logging in case SDL_LoadFunction fail).

I don't understand why it doesn't compile for you. SDL_hidapi.c includes SDL_internals.h which include SDL_Log.h, and SDL_log.h defines SDL_LOG_CATEGORY_INPUT.

On 2020-07-06 10:28:31 +0000, Sven-Hendrik Haase wrote:

That seems to be working well for me, cool!

On 2020-07-10 23:01:23 +0000, Sven-Hendrik Haase wrote:

A user reports that while it doesn't crash, it also doesn't work. I'll try to get them to participate here.

On 2020-07-13 20:06:24 +0000, Dom wrote:

This looks like an important patch to make using HIDAPI less risky

On 2020-12-09 03:04:31 +0000, Sam Lantinga wrote:

Patch added, thanks!
https://hg.libsdl.org/SDL/rev/26a87784645c