This bug report was migrated from our old Bugzilla tracker.

Reported in version: 2.0.8
Reported for operating system, platform: OpenBSD, PowerPC

Comments on the original bug report:

On 2018-10-20 22:38:14 +0000, George Koehler wrote:

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.

On 2018-10-21 00:50:14 +0000, Ozkan Sezer wrote:

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?

On 2018-10-21 01:36:34 +0000, Ryan C. Gordon wrote:

(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.

On 2018-10-21 05:58:13 +0000, George Koehler wrote:

I confirm that 9dd351b26dcb seems to work on my big-endian macppc system. I built that commit and ran a few test programs.