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 5283

Summary: [PATCH] Infinite recursion in X11_SafetyNetErrHandler
Product: SDL Reporter: Cameron Gutman <cameron.gutman>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: major    
Priority: P2    
Version: 2.0.12   
Hardware: x86_64   
OS: Linux   
Attachments: Patch

Description Cameron Gutman 2020-09-08 03:11:39 UTC 
When the video subsystem is initialized 2 or more times (SDL_VideoInit(), SDL_VideoQuit(), SDL_VideoInit() or similar pattern), the X11_SafetyNetErrHandler callback is registered twice. This happens because X11_CreateDevice() always registers X11_SafetyNetErrHandler but X11_DeleteDevice() never restores orig_x11_errhandler.

The second time X11_CreateDevice() is called, orig_x11_errhandler becomes X11_SafetyNetErrHandler (since we have the last handler registration). When X11_SafetyNetErrHandler tries to call the next callback in the chain, it invokes itself and infinite recursion ensues.

p orig_x11_errhandler
$19 = (int (*)(Display *, XErrorEvent *)) 0x7ffff7bb8b90 <X11_SafetyNetErrHandler>

bt
#0  X11_SafetyNetErrHandler (d=0xf62eb0, e=0x7ffee77fcb20) at /usr/src/debug/SDL2-2.0.12-1.fc32.x86_64/src/video/x11/SDL_x11video.c:149
#1  0x00007ffff64c863b in _XError () at /usr/lib64/libX11.so.6
#2  0x00007ffff64c53a7 in handle_error () at /usr/lib64/libX11.so.6
#3  0x00007ffff64c65b3 in _XReply () at /usr/lib64/libX11.so.6
#4  0x00007ffff6478d55 in VA_DRI2GetBuffers_internal () at /usr/lib64/libva-x11.so.2
#5  0x00007ffff6479514 in VA_DRI2GetBuffers () at /usr/lib64/libva-x11.so.2
#6  0x00007ffff6478895 in dri2GetRenderingBuffer () at /usr/lib64/libva-x11.so.2
Comment 1 Cameron Gutman 2020-09-08 03:12:46 UTC 
Created attachment 4460 [details]
Patch
Comment 2 Sam Lantinga 2020-09-08 15:38:44 UTC 
Agreed that this is a problem, but your patch declares the original handler twice and never sets it?
Comment 3 Cameron Gutman 2020-09-08 15:45:43 UTC 
My patch just moved the declaration of orig_x11_errhandler (to be above X11_DeleteDevice() where it is now referenced). It already existed before and is already set in X11_CreateDevice().
Comment 4 Sam Lantinga 2020-09-13 22:05:07 UTC 
Looks good, thanks!
https://hg.libsdl.org/SDL/rev/e0220c4bc306