[issue12287] ossaudiodev: stack corruption with FD >= FD_SETSIZE
Charles-François Natali
report at bugs.python.org
Thu Jun 9 20:13:23 CEST 2011
Charles-François Natali <neologix at free.fr> added the comment:
> You don't check that 0 <= fd (e.g. oss.close()).
>
Actually, none of ossaudiodev's method does...
This makes it even easier to trigger a segfault (at least on RHEL4):
"""
import ossaudiodev
o = ossaudiodev.open('/dev/dsp', 'w')
o.close()
o.writeall(b'toto')
"""
I've attached a patch to fix that (check that the underlying FD isn't closed).
> The select has a specific code for Visual Studio (don't check v < FD_SETSIZE):
> Python has a _PyVerify_fd() function. We might write a similar function/macro to check if a file descriptor can be used in a file descriptor set. FD_SET() is used in the oss, readline, socket and _ssl modules. The socket module has a IS_SELECTABLE() macro:
So, this _PyCheckSelectable_fd ? function/macro would:
- return true (1) on Visual Studio or if
Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE is defined
- return false (0) if the file descriptor is greater than FD_SETSIZE otherwise
Do we agree on that?
Where should I add it? selectmodule, posixmodule, somewhere else?
>
> Note: do you really use the OSS module? On which OS? :)
>
Well, while we don't use ossaudiodev, we have a couple hundred Linux
machines at work, and we use OSS on Linux 2.6.9 kernels (and Python
2.3.4 ;-) )
----------
Added file: http://bugs.python.org/file22301/oss_check_closed.diff
_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue12287>
_______________________________________
-------------- next part --------------
diff -r 964d0d65a2a9 Lib/test/test_ossaudiodev.py
--- a/Lib/test/test_ossaudiodev.py Wed Jun 08 19:18:14 2011 +0200
+++ b/Lib/test/test_ossaudiodev.py Thu Jun 09 20:08:30 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 964d0d65a2a9 Modules/ossaudiodev.c
--- a/Modules/ossaudiodev.c Wed Jun 08 19:18:14 2011 +0200
+++ b/Modules/ossaudiodev.c Thu Jun 09 20:08:30 2011 +0200
@@ -213,6 +213,20 @@
* 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 _check_fd_valid(int fd)
+{
+ if (fd >= 0) {
+ return 1;
+ } else {
+ PyErr_SetString(PyExc_ValueError,
+ "Operation on closed file descriptor");
+ 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 +314,9 @@
static PyObject *
oss_nonblock(oss_audio_t *self, PyObject *unused)
{
+ if (!_check_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 +328,9 @@
static PyObject *
oss_setfmt(oss_audio_t *self, PyObject *args)
{
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
return _do_ioctl_1(self->fd, args, "setfmt", SNDCTL_DSP_SETFMT);
}
@@ -318,6 +338,10 @@
oss_getfmts(oss_audio_t *self, PyObject *unused)
{
int mask;
+
+ if (!_check_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 +350,45 @@
static PyObject *
oss_channels(oss_audio_t *self, PyObject *args)
{
+ if (!_check_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 (!_check_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 (!_check_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 (!_check_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 (!_check_fd_valid(self->fd))
+ return NULL;
+
return _do_ioctl_0(self->fd, args, "post", SNDCTL_DSP_POST);
}
@@ -364,6 +403,9 @@
char *cp;
PyObject *rv;
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (!PyArg_ParseTuple(args, "i:read", &size))
return NULL;
rv = PyBytes_FromStringAndSize(NULL, size);
@@ -391,6 +433,9 @@
char *cp;
int rv, size;
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (!PyArg_ParseTuple(args, "y#:write", &cp, &size)) {
return NULL;
}
@@ -422,6 +467,9 @@
mode, the behaviour of write() and writeall() from Python is
indistinguishable. */
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (!PyArg_ParseTuple(args, "y#:write", &cp, &size))
return NULL;
@@ -489,6 +537,9 @@
static PyObject *
oss_fileno(oss_audio_t *self, PyObject *unused)
{
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
return PyLong_FromLong(self->fd);
}
@@ -503,6 +554,9 @@
int fmt, channels, rate;
PyObject * rv; /* return tuple (fmt, channels, rate) */
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (!PyArg_ParseTuple(args, "iii|i:setparameters",
&wanted_fmt, &wanted_channels, &wanted_rate,
&strict))
@@ -593,6 +647,9 @@
audio_buf_info ai;
int nchannels=0, ssize=0;
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (_ssize(self, &nchannels, &ssize) < 0 || !nchannels || !ssize) {
PyErr_SetFromErrno(PyExc_IOError);
return NULL;
@@ -612,6 +669,9 @@
audio_buf_info ai;
int nchannels=0, ssize=0;
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (_ssize(self, &nchannels, &ssize) < 0 || !nchannels || !ssize) {
PyErr_SetFromErrno(PyExc_IOError);
return NULL;
@@ -632,6 +692,9 @@
audio_buf_info ai;
int nchannels=0, ssize=0;
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (_ssize(self, &nchannels, &ssize) < 0 || !nchannels || !ssize) {
PyErr_SetFromErrno(PyExc_IOError);
return NULL;
@@ -649,6 +712,9 @@
count_info info;
int req;
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
if (self->mode == O_RDONLY)
req = SNDCTL_DSP_GETIPTR;
else
@@ -679,6 +745,9 @@
static PyObject *
oss_mixer_fileno(oss_mixer_t *self, PyObject *unused)
{
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
return PyLong_FromLong(self->fd);
}
@@ -687,6 +756,9 @@
static PyObject *
oss_mixer_controls(oss_mixer_t *self, PyObject *args)
{
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
return _do_ioctl_1_internal(self->fd, args, "controls",
SOUND_MIXER_READ_DEVMASK);
}
@@ -694,6 +766,9 @@
static PyObject *
oss_mixer_stereocontrols(oss_mixer_t *self, PyObject *args)
{
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
return _do_ioctl_1_internal(self->fd, args, "stereocontrols",
SOUND_MIXER_READ_STEREODEVS);
}
@@ -701,6 +776,9 @@
static PyObject *
oss_mixer_reccontrols(oss_mixer_t *self, PyObject *args)
{
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
return _do_ioctl_1_internal(self->fd, args, "reccontrols",
SOUND_MIXER_READ_RECMASK);
}
@@ -710,6 +788,9 @@
{
int channel, volume;
+ if (!_check_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 +811,9 @@
{
int channel, volume, leftVol, rightVol;
+ if (!_check_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 +839,9 @@
static PyObject *
oss_mixer_get_recsrc(oss_mixer_t *self, PyObject *args)
{
+ if (!_check_fd_valid(self->fd))
+ return NULL;
+
return _do_ioctl_1_internal(self->fd, args, "get_recsrc",
SOUND_MIXER_READ_RECSRC);
}
@@ -762,6 +849,9 @@
static PyObject *
oss_mixer_set_recsrc(oss_mixer_t *self, PyObject *args)
{
+ if (!_check_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