Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

segfault by some joystick events #607

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

segfault by some joystick events #607

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 1.2.14
Reported for operating system, platform: Linux, Other

Comments on the original bug report:

On 2010-11-25 14:14:10 +0000, Markus Rathgeb wrote:

First some informations about the used system.
Some informations are perhaps distribution specifig, but I believe it is better to support more information as less...

Distribution: gentoo

[I] media-libs/libsdl
Available versions: 1.2.13-r1 ()1.2.14 ()1.2.14-r1 ()1.2.14-r2 ()1.2.14-r3 (~)1.2.14-r4 {X aalib alsa +audio custom-cflags dga directfb esd fbcon ggi +joystick libcaca nas opengl oss ps3 pulseaudio static-libs svga tslib +video xinerama xv}
Installed versions: 1.2.14-r4(11:06:54 PM 11/25/2010)(X alsa audio directfb fbcon joystick opengl pulseaudio video xinerama xv -aalib -custom-cflags -dga -ggi -libcaca -nas -oss -ps3 -static-libs -svga -tslib)
Homepage: http://www.libsdl.org/
Description: Simple Direct Media Layer

To test joystick (but the same result will occur if I use e.g. sdlmame):

[I] games-misc/sdljoytest
Available versions: (~)11102003
Installed versions: 11102003(10:31:49 PM 11/25/2010)
Homepage: http://sdljoytest.sourceforge.net/
Description: SDL app to test joysticks and game controllers


maggu2810@thor /tmp/SDL-1.2.14 $ gdb SDLJoytest-GL
GNU gdb (Gentoo 7.2 p1) 7.2
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
For bug reporting instructions, please see:
http://bugs.gentoo.org/...
Reading symbols from /usr/bin/SDLJoytest-GL...Reading symbols from /usr/lib/debug/usr/bin/SDLJoytest-GL.debug...done.
done.
(gdb) run -j 0
Starting program: /usr/bin/SDLJoytest-GL -j 0
[Thread debugging using libthread_db enabled]
Found 2 joystick(s) on this system.
Joystick 0
Name:.........GreenAsia Inc. USB Joystick
Axes: 1
TrackBalls:...0
Buttons: 12
Joystick 1
Name:.........GreenAsia Inc. USB Joystick
Axes: 1
TrackBalls:...0
Buttons: 12
Joystick 0:GreenAsia Inc. USB Joystick selected.
Screen resolution set to: 640x480x32bpp

Program received signal SIGSEGV, Segmentation fault.
HandleHat (joystick=0x8069880) at ./src/joystick/linux/SDL_sysjoystick.c:939
939 if ( value != the_hat->axis[axis] ) {

===

(gdb) thread apply all bt full

Thread 1 (Thread 0xb5f78710 (LWP 18858)):

0 HandleHat (joystick=0x8069880) at ./src/joystick/linux/SDL_sysjoystick.c:939

    the_hat = 0x0
    position_map = {"\t\001\003", "\b\000\002", "\f\004\006"}

1 EV_HandleEvents (joystick=0x8069880) at ./src/joystick/linux/SDL_sysjoystick.c:1103

    events = {{time = {tv_sec = 1290723012, tv_usec = 560503}, type = 3, code = 16, value = -1}, {time = {tv_sec = 136765668, tv_usec = -1237775669}, type = 8180, code = 46663, value = 136765764}, {
        time = {tv_sec = -1073748536, tv_usec = -1073749112}, type = 5, code = 0, value = 56}, {time = {tv_sec = 2, tv_usec = 8}, type = 51045, code = 46633, value = 134658664}, {time = {
          tv_sec = 134658696, tv_usec = -1209797672}, type = 46788, code = 47073, value = -1073749096}, {time = {tv_sec = 3, tv_usec = -1073748776}, type = 58161, code = 47066, value = -1210401861}, {
        time = {tv_sec = 48, tv_usec = -1073748776}, type = 53236, code = 46634, value = 134658576}, {time = {tv_sec = 1, tv_usec = -1073748952}, type = 4096, code = 0, value = 134658664}, {time = {
          tv_sec = -1208908493, tv_usec = -1073748952}, type = 55988, code = 46633, value = 6}, {time = {tv_sec = 134658664, tv_usec = 4096}, type = 0, code = 0, value = 8}, {time = {tv_sec = 1, 
          tv_usec = 134658576}, type = 45256, code = 2054, value = 134658612}, {time = {tv_sec = 134662788, tv_usec = 134662812}, type = 58376, code = 49151, value = -1}, {time = {tv_sec = 134658664, 
          tv_usec = -1238781141}, type = 58424, code = 49151, value = -1238775127}, {time = {tv_sec = -1238708236, tv_usec = -1073748936}, type = 47501, code = 46633, value = -1208929401}, {time = {
          tv_sec = 134662820, tv_usec = -1208924295}, type = 58440, code = 49151, value = -1208929401}, {time = {tv_sec = -1073748920, tv_usec = -1238772469}, type = 53236, code = 46634, 
        value = 134658576}, {time = {tv_sec = 0, tv_usec = -1073748888}, type = 56270, code = 46633, value = -1208929401}, {time = {tv_sec = 1, tv_usec = -1208924295}, type = 45256, code = 2054, 
        value = 134658588}, {time = {tv_sec = 0, tv_usec = -1238774323}, type = 53236, code = 46634, value = 0}, {time = {tv_sec = -1073748788, tv_usec = -1073748840}, type = 53118, code = 46633, 
        value = 134658588}, {time = {tv_sec = -1073748792, tv_usec = -1073748840}, type = 34658, code = 46649, value = 134658576}, {time = {tv_sec = 134656200, tv_usec = -1073748792}, type = 47644, 
        code = 2054, value = 134656200}, {time = {tv_sec = -1073748776, tv_usec = -1238774005}, type = 8180, code = 46663, value = 134656200}, {time = {tv_sec = -1073748792, tv_usec = -1073748760}, 
        type = 37983, code = 46649, value = 134658576}, {time = {tv_sec = 104, tv_usec = -1073748788}, type = 58568, code = 49151, value = 134658576}, {time = {tv_sec = 134658576, tv_usec = 134970448}, 
        type = 58572, code = 49151, value = 0}, {time = {tv_sec = 1, tv_usec = -1073748776}, type = 0, code = 0, value = -1213041064}, {time = {tv_sec = -1237984133, tv_usec = -1237740525}, type = 8180, 
        code = 46663, value = 134656200}, {time = {tv_sec = 0, tv_usec = -1073748728}, type = 38803, code = 46649, value = 134658576}, {time = {tv_sec = -1208047390, tv_usec = -1237742325}, 
        type = 38666, code = 46649, value = -1236852748}, {time = {tv_sec = 134656200, tv_usec = -1073748696}, type = 38949, code = 46649, value = 134656200}, {time = {tv_sec = 1, tv_usec = 0}, 
        type = 38890, code = 46649, value = -1236852748}}
    i = 0
    len = <value optimized out>
    code = <value optimized out>

2 SDL_SYS_JoystickUpdate (joystick=0x8069880) at ./src/joystick/linux/SDL_sysjoystick.c:1145

    i = <value optimized out>

3 0xb7f58e10 in SDL_JoystickUpdate () at ./src/joystick/SDL_joystick.c:543

No locals.

4 0xb7f30c7e in SDL_PumpEvents () at ./src/events/SDL_events.c:385

    video = <value optimized out>

5 0xb7f31117 in SDL_PollEvent (event=0xbfffe71c) at ./src/events/SDL_events.c:395

No locals.

6 0x080494a8 in ProcessEvent (event=0xbfffe71c, joystick=0x8069880) at main.c:98

    done = 0

7 0x08049aac in main (argc=3, argv=0xbfffe7f4) at main.c:287

    event = {type = 1 '\001', active = {type = 1 '\001', gain = 0 '\000', state = 1 '\001'}, key = {type = 1 '\001', which = 0 '\000', state = 1 '\001', keysym = {scancode = 0 '\000', 
          sym = SDLK_UNKNOWN, mod = KMOD_NONE, unicode = 0}}, motion = {type = 1 '\001', which = 0 '\000', state = 1 '\001', x = 0, y = 0, xrel = 0, yrel = 0}, button = {type = 1 '\001', 
        which = 0 '\000', button = 1 '\001', state = 0 '\000', x = 0, y = 0}, jaxis = {type = 1 '\001', which = 0 '\000', axis = 1 '\001', value = 0}, jball = {type = 1 '\001', which = 0 '\000', 
        ball = 1 '\001', xrel = 0, yrel = 0}, jhat = {type = 1 '\001', which = 0 '\000', hat = 1 '\001', value = 0 '\000'}, jbutton = {type = 1 '\001', which = 0 '\000', button = 1 '\001', 
        state = 0 '\000'}, resize = {type = 1 '\001', w = 0, h = 0}, expose = {type = 1 '\001'}, quit = {type = 1 '\001'}, user = {type = 1 '\001', code = 0, data1 = 0x0, data2 = 0x0}, syswm = {
        type = 1 '\001', msg = 0x0}}
    joystick = 0x8069880

===

(gdb) print value
$1 = 0


"lsusb" states:
"Bus 006 Device 007: ID 0e8f:0012 GreenAsia Inc. Joystick"


uname -a

Linux thor 2.6.36-gentoo # 1 SMP Fri Oct 22 14:02:45 CEST 2010 i686 AMD Athlon(tm) 7750 Dual-Core Processor AuthenticAMD GNU/Linux


$ grep GREEN /usr/src/linux/.config
CONFIG_HID_GREENASIA=m
CONFIG_GREENASIA_FF=y

On 2010-11-25 14:20:36 +0000, Markus Rathgeb wrote:

Sorry, but IMHO this is a distribution problem.

I recompiled libsdl with changed compiler flags ("-O0" instead of "-O2") and the problem did gone.

Could someone confirm that there could be a problem with optimization?
So I could submit a bug entry to the distribution bugzilla.

On 2011-01-23 13:55:51 +0000, Markus Rathgeb wrote:

Hello again!
So, I believe I found the bug in the code.

Have a look at the file
"SDL-1.2.14/src/joystick/linux/SDL_sysjoystick.c"

and the the function
static SDL_bool EV_ConfigJoystick(SDL_Joystick *joystick, int fd)

absbit is defined as
"unsigned long absbit[NBITS(ABS_MAX)]"

and filled with values by
"ioctl(fd, EVIOCGBIT(EV_ABS, sizeof(absbit)), absbit)"

I realized, that with "-O2" the value of absbit[0] changed after the line
"if ( ioctl(fd, EVIOCGABS(i), values) < 0 )"

The placement in memory of "values" and "absbit" differs by 4 byte if I changed between -O0 and -O2.

So I that test code:

int dummy_i;
int dummy[15];
for (dummy_i = 0; dummy_i < 15; ++dummy_i)
dummy[dummy_i] = 0xAFFEDEAD;
ioctl(fd, EVIOCGABS(i), &dummy[5]);
for (dummy_i = 0; dummy_i < 15; ++dummy_i)
printf("dummy[%d] = [0x%X]\n", dummy_i, dummy[dummy_i]);

So here we can see, that six values are written:
dummy[0] = [0xAFFEDEAD]
dummy[1] = [0xAFFEDEAD]
dummy[2] = [0xAFFEDEAD]
dummy[3] = [0xAFFEDEAD]
dummy[4] = [0xAFFEDEAD]
dummy[5] = [0x80]
dummy[6] = [0x0]
dummy[7] = [0xFF]
dummy[8] = [0x0]
dummy[9] = [0xF]
dummy[10] = [0x0]
dummy[11] = [0xAFFEDEAD]
dummy[12] = [0xAFFEDEAD]
dummy[13] = [0xAFFEDEAD]
dummy[14] = [0xAFFEDEAD]

"int values[5];" should be changed to "int values[6];" in that case.

I will test this change and report again with a patch if the test succeed.

On 2011-01-23 14:34:23 +0000, Markus Rathgeb wrote:

With kernel 2.6.31 the struct input_absinfo defined in linux/input.h changed.
A field "__s32 resolution" was added at the end of the struct.

Because the macro EVIOCGABS(abs) is using the struct input_absinfo, it would be better (IMHO) to change the declaration of variable values to
"int values[sizeof(struct input_absinfo) / sizeof(int)];" or using "struct input_absinfo" directly.

On 2011-01-23 14:35:58 +0000, Markus Rathgeb wrote:

Created attachment 557
Use values with generic size

On 2011-01-23 14:37:21 +0000, Markus Rathgeb wrote:

Created attachment 558
Use struct input_absinfo

Here the struct input_absinfo (linux/input.h) will be used, so the size in kernel could change. IMHO better to maintain.

On 2011-01-23 14:42:10 +0000, Markus Rathgeb wrote:

The joysticks are not working, but the segfault are gone away and IMHO the patch / fix should be applied.
I will done further tests if I have any time again.

Could I get a short response what are your opinion about that topic / patches?

On 2011-01-23 15:13:06 +0000, Jen Spradlin wrote:

That last patch looks great.

Do you give me permission to release your code with SDL 1.3 and future versions of SDL under both the LGPL and a closed-source commercial license?

On 2011-01-24 12:24:18 +0000, Markus Rathgeb wrote:

The permission is granted.

But before you should apply the patch, there is one thing I have to proof - perhaps you do know it...

Could it be a bad idea to use a pointer to the struct, because the kernel space did not know how the struct (created in user space) is arranged in memory?
A special compiler could fill the struct with gaps or do another optimization.

In that case an array of integer values is more portable.

I have to read more about the user<->kernel space gate, but perhaps you can explain me if it could be a problem.

On 2011-01-24 14:37:32 +0000, Sam Lantinga wrote:

I double checked the kernel code and the structure, and that definitely looks like the right way to do it.

Thanks!

Your patch is in the repository:
http://hg.libsdl.org/SDL/rev/ac2c68eb1bb9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant