We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4486 - Segfault when pressing a trigger on the Steam Controller (Linux)
Summary: Segfault when pressing a trigger on the Steam Controller (Linux)
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: joystick (show other bugs)
Version: HG 2.1
Hardware: x86 Linux
: P2 critical
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-02 16:37 UTC by Matteo Beniamino
Modified: 2019-06-12 19:30 UTC (History)
1 user (show)

See Also:

Attachments
gdb log of a segmentation fault (4.59 KB, text/x-log)
2019-02-02 16:37 UTC, Matteo Beniamino 		Details
Always allocate max number of hats (579 bytes, patch)
2019-02-02 17:04 UTC, Matteo Beniamino 		Details | Diff
Map hats indices to the correct hwdata_hat element (1.83 KB, patch)
2019-02-03 09:11 UTC, Matteo Beniamino 		Details | Diff
gdb backtrace (6.67 KB, text/x-log)
2019-06-12 16:53 UTC, Matteo Beniamino 		Details
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Matteo Beniamino 2019-02-02 16:37:44 UTC 
Created attachment 3592 [details]
gdb log of a segmentation fault

Pressing a trigger button on a Steam Controller causes a segmentation fault both with stable version and latest mercurial head on Linux. I'm using the recent hid_steam kernel module with lizard_mode disabled (that is no keyboard/mouse emulation). I suspect this is what's happening: the driver exposes two hats. The two hats have indices 0 and 2. Inside linux/SDL_sysjoystick.c two hats are allocated in allocate_hatdata for joystick->hwdata->hats. In HandleHat function the hat parameter (that can be 2) is directly used as the index of the array that only has two elements, causing an out of bounds access. SDL is not expecting to have "holes" between hats indices.

The index 2 is calculated in HandleInputEvents() as (ABS_HAT2X - ABS_HAT0X) / 2 where ABS_HAT2X is the value associated to the hat inside the hid_steam module.
Comment 1 Matteo Beniamino 2019-02-02 17:04:20 UTC 
Created attachment 3593 [details]
Always allocate max number of hats

I have a workaround: always allocate the maximum number of hats. At first I thought it was a ugly hack, but then I realized that currently said maximum number is 4 and the struct to be allocated is just an array of two ints, so it's not a big deal. I let you establish if this is ok or if you want to write a different fix.
Comment 2 Matteo Beniamino 2019-02-03 09:11:30 UTC 
Created attachment 3594 [details]
Map hats indices to the correct hwdata_hat element

A better patch. Allocate one hwdata_hat for each hat, report the correct number of hats (not always 4 like the previous patch did) and keep a map from the index of the hat derived from the device to the correct hwdata_hat element in the SDL array of hats.
Comment 3 Sam Lantinga 2019-06-12 03:37:31 UTC 
I think this is fixed in the latest SDL snapshot, can you verify?
http://www.libsdl.org/tmp/SDL-2.0.zip
Comment 4 Matteo Beniamino 2019-06-12 16:53:25 UTC 
Created attachment 3823 [details]
gdb backtrace

Unfortunately I can still reproduce the crash.
Comment 5 Sam Lantinga 2019-06-12 17:33:26 UTC 
This change looks good, thanks!
https://hg.libsdl.org/SDL/rev/c54ce7eddcbe
Comment 6 Sam Lantinga 2019-06-12 17:36:44 UTC 
Actually, I think this change is needed too, can you verify?
https://hg.libsdl.org/SDL/rev/baae9331abc0
Comment 7 Sam Lantinga 2019-06-12 17:39:43 UTC 
Better, for clarity:
https://hg.libsdl.org/SDL/rev/a9a7c1e48f0b
Comment 8 Matteo Beniamino 2019-06-12 18:20:32 UTC 
Yes, I can confirm no more crashes using a9a7c1e48f0b !

Thank you.
Comment 9 Sam Lantinga 2019-06-12 19:30:31 UTC 
Great, thanks!