Skip to content
Permalink
Browse files

Fixed bug 2330 - Debian bug report: SDL2 X11 driver buffer overflow w…

…ith 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.
  • Loading branch information
slouken committed Aug 15, 2017
1 parent 9451cd8 commit fb835f9e3b897ef48167d7aff3ccb024d087b745
@@ -662,7 +662,7 @@ if(LIBC)
_uitoa _ultoa strtol strtoul _i64toa _ui64toa strtoll strtoull
atoi atof strcmp strncmp _stricmp strcasecmp _strnicmp strncasecmp
vsscanf vsnprintf fopen64 fseeko fseeko64 sigaction setjmp
nanosleep sysconf sysctlbyname getauxval
nanosleep sysconf sysctlbyname getauxval poll
)
string(TOUPPER ${_FN} _UPPER)
set(_HAVEVAR "HAVE_${_UPPER}")
@@ -16631,7 +16631,7 @@ fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi

for ac_func in malloc calloc realloc free getenv setenv putenv unsetenv qsort abs bcopy memset memcpy memmove wcslen wcslcpy wcslcat wcscmp strlen strlcpy strlcat strdup _strrev _strupr _strlwr strchr strrchr strstr itoa _ltoa _uitoa _ultoa strtol strtoul _i64toa _ui64toa strtoll strtoull atoi atof strcmp strncmp _stricmp strcasecmp _strnicmp strncasecmp vsscanf vsnprintf fopen64 fseeko fseeko64 sigaction setjmp nanosleep sysconf sysctlbyname getauxval
for ac_func in malloc calloc realloc free getenv setenv putenv unsetenv qsort abs bcopy memset memcpy memmove wcslen wcslcpy wcslcat wcscmp strlen strlcpy strlcat strdup _strrev _strupr _strlwr strchr strrchr strstr itoa _ltoa _uitoa _ultoa strtol strtoul _i64toa _ui64toa strtoll strtoull atoi atof strcmp strncmp _stricmp strcasecmp _strnicmp strncasecmp vsscanf vsnprintf fopen64 fseeko fseeko64 sigaction setjmp nanosleep sysconf sysctlbyname getauxval poll
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
if test x$use_input_events = xyes; then
SOURCES="$SOURCES $srcdir/src/core/linux/SDL_evdev*.c"
fi
# Set up other core UNIX files
SOURCES="$SOURCES $srcdir/src/core/unix/*.c"
;;
*-*-cygwin* | *-*-mingw32*)
ARCH=win32
@@ -268,7 +268,7 @@ if test x$enable_libc = xyes; then
AC_DEFINE(HAVE_MPROTECT, 1, [ ])
]),
)
AC_CHECK_FUNCS(malloc calloc realloc free getenv setenv putenv unsetenv qsort abs bcopy memset memcpy memmove wcslen wcslcpy wcslcat wcscmp strlen strlcpy strlcat strdup _strrev _strupr _strlwr strchr strrchr strstr itoa _ltoa _uitoa _ultoa strtol strtoul _i64toa _ui64toa strtoll strtoull atoi atof strcmp strncmp _stricmp strcasecmp _strnicmp strncasecmp vsscanf vsnprintf fopen64 fseeko fseeko64 sigaction setjmp nanosleep sysconf sysctlbyname getauxval)
AC_CHECK_FUNCS(malloc calloc realloc free getenv setenv putenv unsetenv qsort abs bcopy memset memcpy memmove wcslen wcslcpy wcslcat wcscmp strlen strlcpy strlcat strdup _strrev _strupr _strlwr strchr strrchr strstr itoa _ltoa _uitoa _ultoa strtol strtoul _i64toa _ui64toa strtoll strtoull atoi atof strcmp strncmp _stricmp strcasecmp _strnicmp strncasecmp vsscanf vsnprintf fopen64 fseeko fseeko64 sigaction setjmp nanosleep sysconf sysctlbyname getauxval poll)

AC_CHECK_LIB(m, pow, [LIBS="$LIBS -lm"; EXTRA_LDFLAGS="$EXTRA_LDFLAGS -lm"])
AC_CHECK_FUNCS(atan atan2 acos asin ceil copysign cos cosf fabs floor log pow scalbn sin sinf sqrt sqrtf tan tanf)
@@ -3352,6 +3352,8 @@ case "$host" in
if test x$use_input_events = xyes; then
SOURCES="$SOURCES $srcdir/src/core/linux/SDL_evdev*.c"
fi
# Set up other core UNIX files
SOURCES="$SOURCES $srcdir/src/core/unix/*.c"
;;
*-*-cygwin* | *-*-mingw32*)
ARCH=win32
@@ -181,6 +181,7 @@
#cmakedefine HAVE_PTHREAD_SET_NAME_NP 1
#cmakedefine HAVE_SEM_TIMEDWAIT 1
#cmakedefine HAVE_GETAUXVAL 1
#cmakedefine HAVE_POLL 1

#elif __WIN32__
#cmakedefine HAVE_STDARG_H 1
@@ -183,6 +183,7 @@
#undef HAVE_PTHREAD_SET_NAME_NP
#undef HAVE_SEM_TIMEDWAIT
#undef HAVE_GETAUXVAL
#undef HAVE_POLL

#else
#define HAVE_STDARG_H 1
@@ -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;
};
@@ -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)";
@@ -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;
};
@@ -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;
@@ -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;
};
@@ -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;

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;
int result;

/* 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
@@ -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
}

@@ -36,3 +36,5 @@ extern void SDL_IME_UpdateTextRect(SDL_Rect *rect);
extern void SDL_IME_PumpEvents(void);

#endif /* SDL_ime_h_ */

/* vi: set ts=4 sw=4 expandtab: */
@@ -31,7 +31,10 @@

#include <linux/input.h>

#include "SDL.h"
#include "SDL_assert.h"
#include "SDL_loadso.h"
#include "SDL_timer.h"
#include "../unix/SDL_poll.h"

static const char *SDL_UDEV_LIBS[] = {
#ifdef SDL_UDEV_DYNAMIC
@@ -105,14 +108,7 @@ SDL_UDEV_hotplug_update_available(void)
{
if (_this->udev_mon != NULL) {
const int fd = _this->udev_monitor_get_fd(_this->udev_mon);
fd_set fds;
struct timeval tv;

FD_ZERO(&fds);
FD_SET(fd, &fds);
tv.tv_sec = 0;
tv.tv_usec = 0;
if ((select(fd+1, &fds, NULL, NULL, &tv) > 0) && (FD_ISSET(fd, &fds))) {
if (SDL_IOReady(fd, SDL_FALSE, 0)) {
return SDL_TRUE;
}
}
@@ -117,3 +117,5 @@ extern void SDL_UDEV_DelCallback(SDL_UDEV_Callback cb);
#endif /* HAVE_LIBUDEV_H */

#endif /* SDL_udev_h_ */

/* vi: set ts=4 sw=4 expandtab: */
@@ -30,6 +30,7 @@

#include "SDL_stdinc.h"
#include "SDL_assert.h"
#include "../../core/unix/SDL_poll.h"

#include "SDL_waylandvideo.h"
#include "SDL_waylanddatamanager.h"
@@ -46,16 +47,8 @@ write_pipe(int fd, const void* buffer, size_t total_length, size_t *pos)
sigset_t sig_set;
sigset_t old_sig_set;
struct timespec zerotime = {0};
fd_set set;
struct timeval timeout;

FD_ZERO(&set);
FD_SET(fd, &set);

timeout.tv_sec = 1;
timeout.tv_usec = 0;

ready = select(fd + 1, NULL, &set, NULL, &timeout);
ready = SDL_IOReady(fd, SDL_TRUE, 1 * 1000);

sigemptyset(&sig_set);
sigaddset(&sig_set, SIGPIPE);
@@ -92,16 +85,7 @@ read_pipe(int fd, void** buffer, size_t* total_length, SDL_bool null_terminate)
ssize_t bytes_read = 0;
size_t pos = 0;

fd_set set;
struct timeval timeout;

FD_ZERO(&set);
FD_SET(fd, &set);

timeout.tv_sec = 1;
timeout.tv_usec = 0;

ready = select(fd + 1, &set, NULL, NULL, &timeout);
ready = SDL_IOReady(fd, SDL_FALSE, 1 * 1000);

if (ready == 0) {
bytes_read = SDL_SetError("Pipe timeout");

0 comments on commit fb835f9

Please sign in to comment.