| Summary: | BOOL versus Bool corrupts stack in X11_InitKeyboard | ||
|---|---|---|---|
| Product: | SDL | Reporter: | George Koehler <xkernigh> |
| Component: | video | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | icculus, sezeroz |
| Version: | 2.0.8 | ||
| Hardware: | PowerPC | ||
| OS: | OpenBSD | ||
This is a good find.
> because there is also a wrong prototype of XkbSetDetectableAutoRepeat
> in SDL_x11sym.h.
That proto needs changing too. Ryan, Sam?
(In reply to Ozkan Sezer from comment #1) > That proto needs changing too. Ryan, Sam? Yep, George is correct. I've now fixed this in https://hg.libsdl.org/SDL/rev/9dd351b26dcb Thanks! --ryan. I confirm that 9dd351b26dcb seems to work on my big-endian macppc system. I built that commit and ran a few test programs. |
X11 made a stupid mistake and defined 2 boolean types, BOOL and Bool, with different sizes. My machine has 1-byte BOOL and 4-byte Bool. src/video/x11/SDL_x11keyboard.c X11_InitKeyboard() declares BOOL xkb_repeat, allocating 1 byte on the stack. It then passes &xkb_repeat to X11_XkbSetDetectableAutoRepeat(), which expects a Bool * pointing to 4 bytes. When XkbSetDetectableAutoRepeat() writes its 0 or 1 to the Bool *, it writes 4 bytes to the 1-byte BOOL xkb_repeat. This overflow corrupts 3 bytes on the stack. If we are lucky, the 3 bytes might be unused padding for alignment. I was unlucky. I have an unusual macppc machine running OpenBSD-current. The compiler is gcc 4.2.1 and enables -fstack-protector by default. The compiler put BOOL xkb_repeat near the stack canary, so XkbSetDetectableAutoRepeat() smashed the canary. When X11_InitKeyboard() returned, it found the dead canary, aborted the program, dumped core, and wrote "stack overflow in function X11_InitKeyboard" to /var/log/messages. This bug caused every program using SDL2 on OpenBSD/macppc to crash when calling SDL_Init() to initialize the video. Yes, every program using SDL2 on OpenBSD/macppc was broken, but macppc machines are old and rarely used, so I don't know if anyone other than me noticed the problem. I made a local fix for OpenBSD's package of SDL 2.0.8: Index: src/video/x11/SDL_x11keyboard.c --- src/video/x11/SDL_x11keyboard.c.orig +++ src/video/x11/SDL_x11keyboard.c @@ -266,7 +266,9 @@ X11_InitKeyboard(_THIS) int best_distance; int best_index; int distance; - BOOL xkb_repeat = 0; + /* This must be Bool from <X11/Xlib.h>, not BOOL from <X11/Xmd.h>, + because the types have different sizes. */ + Bool xkb_repeat = 0; X11_XAutoRepeatOn(data->display); This is enough to prevent the crash and play Simutrans and SuperTux, but isn't a complete fix because there is also a wrong prototype of XkbSetDetectableAutoRepeat in SDL_x11sym.h. I have not yet written and tested a complete fix. <X11/Xlib.h> declares Bool as #define Bool int <X11/Xmd.h> declares BOOL as typedef unsigned char CARD8; typedef CARD8 BOOL; BOOL is an unsigned char of 1 byte. Bool is an int, which has 4 bytes on most machines. Writing through Bool * to a BOOL corrupts 3 bytes of memory.