Fixed bug 2330 - Debian bug report: SDL2 X11 driver buffer overflow with large X11 file descriptor

manuel.montezelo

Original bug report (note that it was against 2.0.0, it might have been fixed in between):  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733015

--------------------------------------------------------
Package: libsdl2-2.0-0
Version: 2.0.0+dfsg1-3
Severity: normal
Tags: patch

I have occasional crashes here caused by the X11 backend of SDL2. It seems to
be caused by the X11_Pending function trying to add a high number (> 1024)
file descriptor to a fd_set before doing a select on it to avoid busy waiting
on X11 events. This causes a buffer overflow because the file descriptor is
larger (or equal) than the limit FD_SETSIZE.

Attached is a possible workaround patch.

Please also keep in mind that fd_set are also used in following files which
may have similar problems.

src/audio/bsd/SDL_bsdaudio.c
src/audio/paudio/SDL_paudio.c
src/audio/qsa/SDL_qsa_audio.c
src/audio/sun/SDL_sunaudio.c
src/joystick/linux/SDL_sysjoystick.c


--------------------------------------------------------

On Tuesday 24 December 2013 00:43:13 Sven Eckelmann wrote:
> I have occasional crashes here caused by the X11 backend of SDL2. It seems
> to be caused by the X11_Pending function trying to add a high number (>
> 1024) file descriptor to a fd_set before doing a select on it to avoid busy
> waiting on X11 events. This causes a buffer overflow because the file
> descriptor is larger (or equal) than the limit FD_SETSIZE.


I personally experienced this problem while hacking on the python bindings
package for SDL2 [1] (while doing make runtest). But it easier to reproduce in
a smaller, synthetic testcase.
This commit is contained in:
Sam Lantinga 2017-08-14 20:22:19 -07:00
parent e97f14a10e
commit 87ea96c003
18 changed files with 63 additions and 144 deletions

View file

@ -42,7 +42,7 @@ struct SDL_PrivateAudioData
Uint8 *mixbuf;
int mixlen;
/* Support for audio timing using a timer, in addition to select() */
/* Support for audio timing using a timer, in addition to SDL_IOReady() */
float frame_ticks;
float next_frame;
};

View file

@ -38,6 +38,7 @@
#include "SDL_timer.h"
#include "SDL_audio.h"
#include "../../core/unix/SDL_poll.h"
#include "../SDL_audio_c.h"
#include "../SDL_audiodev_c.h"
#include "SDL_netbsdaudio.h"
@ -134,18 +135,11 @@ NETBSDAUDIO_WaitDevice(_THIS)
SDL_Delay(ticks);
}
} else {
/* Use select() for audio synchronization */
fd_set fdset;
struct timeval timeout;
FD_ZERO(&fdset);
FD_SET(this->hidden->audio_fd, &fdset);
timeout.tv_sec = 10;
timeout.tv_usec = 0;
/* Use SDL_IOReady() for audio synchronization */
#ifdef DEBUG_AUDIO
fprintf(stderr, "Waiting for audio to get ready\n");
#endif
if (select(this->hidden->audio_fd + 1, NULL, &fdset, NULL, &timeout)
if (SDL_IOReady(this->hidden->audio_fd, SDL_TRUE, 10 * 1000)
<= 0) {
const char *message =
"Audio timeout - buggy audio driver? (disabled)";

View file

@ -36,7 +36,7 @@ struct SDL_PrivateAudioData
Uint8 *mixbuf;
int mixlen;
/* Support for audio timing using a timer, in addition to select() */
/* Support for audio timing using a timer, in addition to SDL_IOReady() */
float frame_ticks;
float next_frame;
};

View file

@ -36,6 +36,7 @@
#include "SDL_audio.h"
#include "SDL_stdinc.h"
#include "../SDL_audio_c.h"
#include "../../core/unix/SDL_poll.h"
#include "SDL_paudio.h"
/* #define DEBUG_AUDIO */
@ -137,44 +138,31 @@ PAUDIO_WaitDevice(_THIS)
SDL_Delay(ticks);
}
} else {
int timeoutMS;
audio_buffer paud_bufinfo;
/* Use select() for audio synchronization */
struct timeval timeout;
FD_ZERO(&fdset);
FD_SET(this->hidden->audio_fd, &fdset);
if (ioctl(this->hidden->audio_fd, AUDIO_BUFFER, &paud_bufinfo) < 0) {
#ifdef DEBUG_AUDIO
fprintf(stderr, "Couldn't get audio buffer information\n");
#endif
timeout.tv_sec = 10;
timeout.tv_usec = 0;
timeoutMS = 10 * 1000;
} else {
long ms_in_buf = paud_bufinfo.write_buf_time;
timeout.tv_sec = ms_in_buf / 1000;
ms_in_buf = ms_in_buf - timeout.tv_sec * 1000;
timeout.tv_usec = ms_in_buf * 1000;
timeoutMS = paud_bufinfo.write_buf_time;
#ifdef DEBUG_AUDIO
fprintf(stderr,
"Waiting for write_buf_time=%ld,%ld\n",
timeout.tv_sec, timeout.tv_usec);
fprintf(stderr, "Waiting for write_buf_time=%d ms\n", timeoutMS);
#endif
}
#ifdef DEBUG_AUDIO
fprintf(stderr, "Waiting for audio to get ready\n");
#endif
if (select(this->hidden->audio_fd + 1, NULL, &fdset, NULL, &timeout)
<= 0) {
const char *message =
"Audio timeout - buggy audio driver? (disabled)";
if (SDL_IOReady(this->hidden->audio_fd, SDL_TRUE, timeoutMS) <= 0) {
/*
* In general we should never print to the screen,
* but in this case we have no other way of letting
* the user know what happened.
*/
fprintf(stderr, "SDL: %s - %s\n", strerror(errno), message);
fprintf(stderr, "SDL: %s - Audio timeout - buggy audio driver? (disabled)\n", strerror(errno));
SDL_OpenedAudioDeviceDisconnected(this);
/* Don't try to close - may hang */
this->hidden->audio_fd = -1;
@ -486,7 +474,7 @@ PAUDIO_OpenDevice(_THIS, void *handle, const char *devname, int iscapture)
return SDL_SetError("Can't start audio play");
}
/* Check to see if we need to use select() workaround */
/* Check to see if we need to use SDL_IOReady() workaround */
if (workaround != NULL) {
this->hidden->frame_ticks = (float) (this->spec.samples * 1000) /
this->spec.freq;

View file

@ -37,7 +37,7 @@ struct SDL_PrivateAudioData
Uint8 *mixbuf;
int mixlen;
/* Support for audio timing using a timer, in addition to select() */
/* Support for audio timing using a timer, in addition to SDL_IOReady() */
float frame_ticks;
float next_frame;
};

View file

@ -45,6 +45,7 @@
#include "SDL_timer.h"
#include "SDL_audio.h"
#include "../../core/unix/SDL_poll.h"
#include "../SDL_audio_c.h"
#include "SDL_qsa_audio.h"
@ -113,67 +114,25 @@ QSA_InitAudioParams(snd_pcm_channel_params_t * cpars)
static void
QSA_WaitDevice(_THIS)
{
fd_set wfds;
fd_set rfds;
int selectret;
struct timeval timeout;
int result;
if (!this->hidden->iscapture) {
FD_ZERO(&wfds);
FD_SET(this->hidden->audio_fd, &wfds);
} else {
FD_ZERO(&rfds);
FD_SET(this->hidden->audio_fd, &rfds);
}
do {
/* Setup timeout for playing one fragment equal to 2 seconds */
/* If timeout occured than something wrong with hardware or driver */
/* For example, Vortex 8820 audio driver stucks on second DAC because */
/* it doesn't exist ! */
timeout.tv_sec = 2;
timeout.tv_usec = 0;
/* Setup timeout for playing one fragment equal to 2 seconds */
/* If timeout occured than something wrong with hardware or driver */
/* For example, Vortex 8820 audio driver stucks on second DAC because */
/* it doesn't exist ! */
result = SDL_IOReady(this->hidden->audio_fd, !this->hidden->iscapture, 2 * 1000);
switch (result) {
case -1:
SDL_SetError("QSA: SDL_IOReady() failed: %s", strerror(errno));
break;
case 0:
SDL_SetError("QSA: timeout on buffer waiting occured");
this->hidden->timeout_on_wait = 1;
break;
default:
this->hidden->timeout_on_wait = 0;
if (!this->hidden->iscapture) {
selectret =
select(this->hidden->audio_fd + 1, NULL, &wfds, NULL,
&timeout);
} else {
selectret =
select(this->hidden->audio_fd + 1, &rfds, NULL, NULL,
&timeout);
}
switch (selectret) {
case -1:
{
SDL_SetError("QSA: select() failed: %s", strerror(errno));
return;
}
break;
case 0:
{
SDL_SetError("QSA: timeout on buffer waiting occured");
this->hidden->timeout_on_wait = 1;
return;
}
break;
default:
{
if (!this->hidden->iscapture) {
if (FD_ISSET(this->hidden->audio_fd, &wfds)) {
return;
}
} else {
if (FD_ISSET(this->hidden->audio_fd, &rfds)) {
return;
}
}
}
break;
}
} while (1);
break;
}
}
static void

View file

@ -40,6 +40,7 @@
#include "SDL_timer.h"
#include "SDL_audio.h"
#include "../../core/unix/SDL_poll.h"
#include "../SDL_audio_c.h"
#include "../SDL_audiodev_c.h"
#include "SDL_sunaudio.h"
@ -97,11 +98,7 @@ SUNAUDIO_WaitDevice(_THIS)
}
}
#else
fd_set fdset;
FD_ZERO(&fdset);
FD_SET(this->hidden->audio_fd, &fdset);
select(this->hidden->audio_fd + 1, NULL, &fdset, NULL, NULL);
SDL_IOReady(this->hidden->audio_fd, SDL_TRUE, -1);
#endif
}