[issue12287] ossaudiodev: stack corruption with FD >= FD_SETSIZE

Charles-François Natali report at bugs.python.org
Fri Jun 10 23:45:28 CEST 2011


Charles-François Natali <neologix at free.fr> added the comment:

Two patches attached:
- a patch checking that the ossaudiodev object isn't closed
- a patch adding _PyIsSelectable_fd()

----------
Added file: http://bugs.python.org/file22324/is_selectable.diff
Added file: http://bugs.python.org/file22325/oss_check_closed.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue12287>
_______________________________________
-------------- next part --------------
diff -r e572a97a1bd1 Include/fileobject.h
--- a/Include/fileobject.h	Fri Jun 10 19:05:16 2011 +0100
+++ b/Include/fileobject.h	Fri Jun 10 23:35:14 2011 +0200
@@ -44,6 +44,15 @@
 #endif
 #endif /* Py_LIMITED_API */
 
+#ifdef HAVE_SELECT
+/* A routine to check if a file descriptor can be select()-ed. */
+#ifdef Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE
+ #define _PyIsSelectable_fd(FD) (1)
+#else
+ #define _PyIsSelectable_fd(FD) (((FD) >= 0) && ((FD) < FD_SETSIZE))
+#endif
+#endif /* HAVE_SELECT */
+
 #ifdef __cplusplus
 }
 #endif
diff -r e572a97a1bd1 Modules/_ssl.c
--- a/Modules/_ssl.c	Fri Jun 10 19:05:16 2011 +0100
+++ b/Modules/_ssl.c	Fri Jun 10 23:35:14 2011 +0200
@@ -1022,10 +1022,8 @@
 #endif
 
     /* Guard against socket too large for select*/
-#ifndef Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE
-    if (s->sock_fd >= FD_SETSIZE)
+    if (!_PyIsSelectable_fd(s->sock_fd))
         return SOCKET_TOO_LARGE_FOR_SELECT;
-#endif
 
     /* Construct the arguments to select */
     tv.tv_sec = (int)s->sock_timeout;
diff -r e572a97a1bd1 Modules/ossaudiodev.c
--- a/Modules/ossaudiodev.c	Fri Jun 10 19:05:16 2011 +0100
+++ b/Modules/ossaudiodev.c	Fri Jun 10 23:35:14 2011 +0200
@@ -425,6 +425,11 @@
     if (!PyArg_ParseTuple(args, "y#:write", &cp, &size))
         return NULL;
 
+    if (!_PyIsSelectable_fd(self->fd)) {
+        PyErr_SetString(PyExc_ValueError,
+                        "file descriptor out of range for select");
+        return NULL;
+    }
     /* use select to wait for audio device to be available */
     FD_ZERO(&write_set_fds);
     FD_SET(self->fd, &write_set_fds);
diff -r e572a97a1bd1 Modules/selectmodule.c
--- a/Modules/selectmodule.c	Fri Jun 10 19:05:16 2011 +0100
+++ b/Modules/selectmodule.c	Fri Jun 10 23:35:14 2011 +0200
@@ -110,7 +110,7 @@
 #if defined(_MSC_VER)
         max = 0;                             /* not used for Win32 */
 #else  /* !_MSC_VER */
-        if (v < 0 || v >= FD_SETSIZE) {
+        if (!_PyIsSelectable_fd(v)) {
             PyErr_SetString(PyExc_ValueError,
                         "filedescriptor out of range in select()");
             goto finally;
@@ -160,13 +160,6 @@
     for (j = 0; fd2obj[j].sentinel >= 0; j++) {
         fd = fd2obj[j].fd;
         if (FD_ISSET(fd, set)) {
-#ifndef _MSC_VER
-            if (fd > FD_SETSIZE) {
-                PyErr_SetString(PyExc_SystemError,
-               "filedescriptor out of range returned in select()");
-                goto finally;
-            }
-#endif
             o = fd2obj[j].obj;
             fd2obj[j].obj = NULL;
             /* transfer ownership */
diff -r e572a97a1bd1 Modules/socketmodule.c
--- a/Modules/socketmodule.c	Fri Jun 10 19:05:16 2011 +0100
+++ b/Modules/socketmodule.c	Fri Jun 10 23:35:14 2011 +0200
@@ -469,18 +469,14 @@
 #include <sys/poll.h>
 #endif
 
-#ifdef Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE
-/* Platform can select file descriptors beyond FD_SETSIZE */
-#define IS_SELECTABLE(s) 1
-#elif defined(HAVE_POLL)
+#ifdef HAVE_POLL
 /* Instead of select(), we'll use poll() since poll() works on any fd. */
 #define IS_SELECTABLE(s) 1
 /* Can we call select() with this socket without a buffer overrun? */
 #else
-/* POSIX says selecting file descriptors beyond FD_SETSIZE
-   has undefined behaviour.  If there's no timeout left, we don't have to
-   call select, so it's a safe, little white lie. */
-#define IS_SELECTABLE(s) ((s)->sock_fd < FD_SETSIZE || s->sock_timeout <= 0.0)
+/* If there's no timeout left, we don't have to call select, so it's a safe,
+ * little white lie. */
+#define IS_SELECTABLE(s) (_PyIsSelectable_fd((s)->sock_fd) || s->sock_timeout <= 0.0)
 #endif
 
 static PyObject*
-------------- next part --------------
diff -r e572a97a1bd1 Lib/test/test_ossaudiodev.py
--- a/Lib/test/test_ossaudiodev.py	Fri Jun 10 19:05:16 2011 +0100
+++ b/Lib/test/test_ossaudiodev.py	Fri Jun 10 23:44:08 2011 +0200
@@ -170,6 +170,14 @@
             pass
         self.assertTrue(dsp.closed)
 
+    def test_on_closed(self):
+        dsp = ossaudiodev.open('w')
+        dsp.close()
+        self.assertRaises(ValueError, dsp.fileno)
+        self.assertRaises(ValueError, dsp.read, 1)
+        self.assertRaises(ValueError, dsp.write, b'x')
+        self.assertRaises(ValueError, dsp.writeall, b'x')
+
 
 def test_main():
     try:
diff -r e572a97a1bd1 Modules/ossaudiodev.c
--- a/Modules/ossaudiodev.c	Fri Jun 10 19:05:16 2011 +0100
+++ b/Modules/ossaudiodev.c	Fri Jun 10 23:44:08 2011 +0200
@@ -213,6 +213,21 @@
  * Helper functions
  */
 
+/* Check if a given file descriptor is valid (i.e. hasn't been closed).
+ * If true, return 1. Otherwise, raise ValueError and return 0.
+ */
+static int _is_fd_valid(int fd)
+{
+    /* the FD is set to -1 in oss_close()/oss_mixer_close() */
+    if (fd >= 0) {
+        return 1;
+    } else {
+        PyErr_SetString(PyExc_ValueError,
+                        "Operation on closed OSS device.");
+        return 0;
+    }
+}
+
 /* _do_ioctl_1() is a private helper function used for the OSS ioctls --
    SNDCTL_DSP_{SETFMT,CHANNELS,SPEED} -- that that are called from C
    like this:
@@ -300,6 +315,9 @@
 static PyObject *
 oss_nonblock(oss_audio_t *self, PyObject *unused)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     /* Hmmm: it doesn't appear to be possible to return to blocking
        mode once we're in non-blocking mode! */
     if (ioctl(self->fd, SNDCTL_DSP_NONBLOCK, NULL) == -1)
@@ -311,6 +329,9 @@
 static PyObject *
 oss_setfmt(oss_audio_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1(self->fd, args, "setfmt", SNDCTL_DSP_SETFMT);
 }
 
@@ -318,6 +339,10 @@
 oss_getfmts(oss_audio_t *self, PyObject *unused)
 {
     int mask;
+
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (ioctl(self->fd, SNDCTL_DSP_GETFMTS, &mask) == -1)
         return PyErr_SetFromErrno(PyExc_IOError);
     return PyLong_FromLong(mask);
@@ -326,30 +351,45 @@
 static PyObject *
 oss_channels(oss_audio_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1(self->fd, args, "channels", SNDCTL_DSP_CHANNELS);
 }
 
 static PyObject *
 oss_speed(oss_audio_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1(self->fd, args, "speed", SNDCTL_DSP_SPEED);
 }
 
 static PyObject *
 oss_sync(oss_audio_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_0(self->fd, args, "sync", SNDCTL_DSP_SYNC);
 }
 
 static PyObject *
 oss_reset(oss_audio_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_0(self->fd, args, "reset", SNDCTL_DSP_RESET);
 }
 
 static PyObject *
 oss_post(oss_audio_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_0(self->fd, args, "post", SNDCTL_DSP_POST);
 }
 
@@ -364,6 +404,9 @@
     char *cp;
     PyObject *rv;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (!PyArg_ParseTuple(args, "i:read", &size))
         return NULL;
     rv = PyBytes_FromStringAndSize(NULL, size);
@@ -391,6 +434,9 @@
     char *cp;
     int rv, size;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (!PyArg_ParseTuple(args, "y#:write", &cp, &size)) {
         return NULL;
     }
@@ -422,6 +468,9 @@
        mode, the behaviour of write() and writeall() from Python is
        indistinguishable. */
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (!PyArg_ParseTuple(args, "y#:write", &cp, &size))
         return NULL;
 
@@ -489,6 +538,9 @@
 static PyObject *
 oss_fileno(oss_audio_t *self, PyObject *unused)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return PyLong_FromLong(self->fd);
 }
 
@@ -503,6 +555,9 @@
     int fmt, channels, rate;
     PyObject * rv;                    /* return tuple (fmt, channels, rate) */
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (!PyArg_ParseTuple(args, "iii|i:setparameters",
                           &wanted_fmt, &wanted_channels, &wanted_rate,
                           &strict))
@@ -593,6 +648,9 @@
     audio_buf_info ai;
     int nchannels=0, ssize=0;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (_ssize(self, &nchannels, &ssize) < 0 || !nchannels || !ssize) {
         PyErr_SetFromErrno(PyExc_IOError);
         return NULL;
@@ -612,6 +670,9 @@
     audio_buf_info ai;
     int nchannels=0, ssize=0;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (_ssize(self, &nchannels, &ssize) < 0 || !nchannels || !ssize) {
         PyErr_SetFromErrno(PyExc_IOError);
         return NULL;
@@ -632,6 +693,9 @@
     audio_buf_info ai;
     int nchannels=0, ssize=0;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (_ssize(self, &nchannels, &ssize) < 0 || !nchannels || !ssize) {
         PyErr_SetFromErrno(PyExc_IOError);
         return NULL;
@@ -649,6 +713,9 @@
     count_info info;
     int req;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     if (self->mode == O_RDONLY)
         req = SNDCTL_DSP_GETIPTR;
     else
@@ -679,6 +746,9 @@
 static PyObject *
 oss_mixer_fileno(oss_mixer_t *self, PyObject *unused)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return PyLong_FromLong(self->fd);
 }
 
@@ -687,6 +757,9 @@
 static PyObject *
 oss_mixer_controls(oss_mixer_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1_internal(self->fd, args, "controls",
         SOUND_MIXER_READ_DEVMASK);
 }
@@ -694,6 +767,9 @@
 static PyObject *
 oss_mixer_stereocontrols(oss_mixer_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1_internal(self->fd, args, "stereocontrols",
         SOUND_MIXER_READ_STEREODEVS);
 }
@@ -701,6 +777,9 @@
 static PyObject *
 oss_mixer_reccontrols(oss_mixer_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1_internal(self->fd, args, "reccontrols",
         SOUND_MIXER_READ_RECMASK);
 }
@@ -710,6 +789,9 @@
 {
     int channel, volume;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     /* Can't use _do_ioctl_1 because of encoded arg thingy. */
     if (!PyArg_ParseTuple(args, "i:get", &channel))
         return NULL;
@@ -730,6 +812,9 @@
 {
     int channel, volume, leftVol, rightVol;
 
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     /* Can't use _do_ioctl_1 because of encoded arg thingy. */
     if (!PyArg_ParseTuple(args, "i(ii):set", &channel, &leftVol, &rightVol))
         return NULL;
@@ -755,6 +840,9 @@
 static PyObject *
 oss_mixer_get_recsrc(oss_mixer_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1_internal(self->fd, args, "get_recsrc",
         SOUND_MIXER_READ_RECSRC);
 }
@@ -762,6 +850,9 @@
 static PyObject *
 oss_mixer_set_recsrc(oss_mixer_t *self, PyObject *args)
 {
+    if (!_is_fd_valid(self->fd))
+        return NULL;
+
     return _do_ioctl_1(self->fd, args, "set_recsrc",
         SOUND_MIXER_WRITE_RECSRC);
 }


More information about the Python-bugs-list mailing list