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 5007

Summary: Segfault in KMSDRM_VideoQuit() on Raspberry Pi Zero with no display attached
Product: SDL Reporter: Charles Huber <genpfault>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: cameron.gutman
Version: HG 2.0   
Hardware: ARM   
OS: Linux   
Attachments: KMSDRM_VideoQuit(): NULL-check dispdata before use

Description Charles Huber 2020-02-28 13:48:47 UTC
See here: https://stackoverflow.com/questions/60432999/

KMSDRM_VideoQuit() should NULL-check the return value of SDL_GetDisplayDriverData() before attempting to dereference it.[1]

[1]: https://hg.libsdl.org/SDL/file/ed7c27865ea7/src/video/kmsdrm/SDL_kmsdrmvideo.c#l664
Comment 1 Charles Huber 2020-03-01 21:51:36 UTC
Got repro on a Raspberry Pi 3B as of today's 13562:e1a1a5a3e551 using 2020-02-13-raspbian-buster.img:

pi@rpi3:~/bug5007 $ SDL_VIDEO_EGL_DRIVER=libEGL.so SDL_VIDEO_GL_DRIVER=libGLESv2.so gdb ./a.out
GNU gdb (Raspbian 8.2.1-2) 8.2.1
<snip>

(gdb) run
Starting program: /home/pi/bug5007/a.out 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
[New Thread 0x76a6b450 (LWP 13136)]

Thread 1 "a.out" received signal SIGSEGV, Segmentation fault.
KMSDRM_VideoQuit (_this=<optimized out>) at /home/pi/SDL/src/video/kmsdrm/SDL_kmsdrmvideo.c:664
664         if (dispdata->conn) {

(gdb) info locals
viddata = 0x298a8
dispdata = 0x0

(gdb) bt
#0  KMSDRM_VideoQuit (_this=<optimized out>) at /home/pi/SDL/src/video/kmsdrm/SDL_kmsdrmvideo.c:664
#1  0x76f2da40 in SDL_VideoQuit_REAL () at /home/pi/SDL/src/video/SDL_video.c:2869
#2  0x76f2e514 in SDL_VideoQuit_REAL () at /home/pi/SDL/src/video/SDL_video.c:2853
#3  SDL_VideoInit_REAL (driver_name=<optimized out>) at /home/pi/SDL/src/video/SDL_video.c:533
#4  0x76e853cc in SDL_InitSubSystem_REAL (flags=62001) at /home/pi/SDL/src/SDL.c:206
#5  0x00010a7c in main ()
Comment 2 Cameron Gutman 2020-03-01 23:08:00 UTC
I'm not 100% sure about the API contract for video drivers in SDL, but it seems odd to me that the video core would call VideoQuit() after VideoInit() failed.

Sam, is that the intended behavior?
Comment 3 Sam Lantinga 2020-03-02 01:57:10 UTC
Yes, it should be safe to call the *Quit() functions anytime.

Charles, can you provide a tested patch? I want to make sure we don't have any other issues in that code path.
Comment 4 Charles Huber 2020-03-02 04:02:27 UTC
Created attachment 4231 [details]
KMSDRM_VideoQuit(): NULL-check dispdata before use

This patch fixes the segfault on my Pi, though the valid display index range reported by the CHECK_DISPLAY_INDEX() macro in src/video/SDL_video.c is a little weird:

$ SDL_VIDEO_EGL_DRIVER=libEGL.so SDL_VIDEO_GL_DRIVER=libGLESv2.so ./a.out
SDL_Init(): displayIndex must be in the range 0 - -1
Comment 5 Sam Lantinga 2020-03-02 22:56:16 UTC
Patch added, thanks!
https://hg.libsdl.org/SDL/rev/d28975f16368