[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset ff6adb867f40 by Charles-François Natali in branch '2.7': Issue #12287: Fix a stack corruption in ossaudiodev module when the FD is http://hg.python.org/cpython/rev/ff6adb867f40 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
STINNER Victor victor.stin...@haypocalc.com added the comment: The _socket module doesn't compile anymore on Windows: Build started: Project: _socket, Configuration: Debug|Win32 Compiling... socketmodule.c 29..\Modules\socketmodule.c(1649) : warning C4013: '_PyIsSelectable_fd' undefined; assuming extern returning int Linking... Creating library d:\cygwin\home\db3l\buildarea\2.7.bolen-windows\build\PCbuild\\_socket_d.lib and object d:\cygwin\home\db3l\buildarea\2.7.bolen-windows\build\PCbuild\\_socket_d.exp socketmodule.obj : error LNK2019: unresolved external symbol __PyIsSelectable_fd referenced in function _sock_accept d:\cygwin\home\db3l\buildarea\2.7.bolen-windows\build\PCbuild\\_socket_d.pyd : fatal error LNK1120: 1 unresolved externals Build log was saved at file://d:\cygwin\home\db3l\buildarea\2.7.bolen-windows\build\PCbuild\Win32-temp-Debug\_socket\BuildLog.htm _socket - 2 error(s), 1 warning(s) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@free.fr added the comment: STINNER Victor victor.stin...@haypocalc.com added the comment: The _socket module doesn't compile anymore on Windows: Fixed (that's why I wanted a Windows expert to have a look at this patch :-). You might replace #if defined(_MSC_VER) with #if defined (MS_WINDOWS), but in another commit. I'd rather not modify code I don't understand. Plus, I have a really poor Windows karma... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 852ca32eb18d by Charles-François Natali in branch '3.2': Issue #12287: Fix a stack corruption in ossaudiodev module when the FD is http://hg.python.org/cpython/rev/852ca32eb18d New changeset ad1c09b6a5b9 by Charles-François Natali in branch 'default': Issue #12287: Fix a stack corruption in ossaudiodev module when the FD is http://hg.python.org/cpython/rev/ad1c09b6a5b9 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@free.fr added the comment: Alright, committed to 2.7, 3.2 an default. Seems to work on all the buildbots, closing. -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@free.fr added the comment: So, what should I do with is_selectable.diff? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Antoine Pitrou pit...@free.fr added the comment: So, what should I do with is_selectable.diff? It looks ok to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
STINNER Victor victor.stin...@haypocalc.com added the comment: You might replace #if defined(_MSC_VER) with #if defined(MS_WINDOWS), but in another commit. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@free.fr added the comment: Brian, any comment about the Windows part (see Victor's message, http://bugs.python.org/issue12287#msg138137) ? -- nosy: +brian.curtin ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@free.fr added the comment: For oss_check_closed.diff, should I apply it to default only or to every branch? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Antoine Pitrou pit...@free.fr added the comment: For oss_check_closed.diff, should I apply it to default only or to every branch? Only default I'd say. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Roundup Robot devnull@devnull added the comment: New changeset d0952a2fb7bd by Charles-François Natali in branch 'default': Issue #12287: In ossaudiodev, check that the device isn't closed in several http://hg.python.org/cpython/rev/d0952a2fb7bd -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Amaury Forgeot d'Arc amaur...@gmail.com added the comment: I don't understand if socket file descriptors are different than (classic) file descriptors. On Windows, sockets are completely independent from file descriptors. A socket id can be large (typically over 1000), fortunately a fd_set is not indexed by descriptors; FD_SET just appends the socket descriptor to the array; there is a limit of 64 sockets, but no limit on the value of a descriptor. -- nosy: +amaury.forgeotdarc ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@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 rep...@bugs.python.org
http://bugs.python.org/issue12287
___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.cFri Jun 10 19:05:16 2011 +0100
+++ b/Modules/_ssl.cFri 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.cFri Jun 10 19:05:16 2011 +0100
+++ b/Modules/selectmodule.cFri 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.cFri Jun 10 19:05:16 2011 +0100
+++ b/Modules/socketmodule.cFri 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*
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
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Changes by Charles-François Natali neolo...@free.fr: Removed file: http://bugs.python.org/file22284/oss_select.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Changes by Charles-François Natali neolo...@free.fr: Removed file: http://bugs.python.org/file22301/oss_check_closed.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Antoine Pitrou pit...@free.fr added the comment: Two patches attached: - a patch checking that the ossaudiodev object isn't closed - a patch adding _PyIsSelectable_fd() In oss_check_closed.diff, you might want to add tests for the other methods as well. Otherwise, looks good! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
STINNER Victor victor.stin...@haypocalc.com added the comment: Comments on is_selectable.diff. +#define IS_SELECTABLE(s) (_PyIsSelectable_fd((s)-sock_fd) || s-sock_timeout = 0.0) You may add parenthesis around the second s (even if it's unrelated to this issue and not a regression of your patch). +#ifdef Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE ... #if defined(_MSC_VER) max = 0; /* not used for Win32 */ I still don't understand these checks: why not testing simply for #ifdef MS_WINDOWS? MinGW or Cygwin have another implementation of select() which is more limited? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@free.fr added the comment: In oss_check_closed.diff, you might want to add tests for the other methods as well. I've added checks for other methods (not all of them, there are quite a few). I've also added a check for the mixer object. You may add parenthesis around the second s Done. I still don't understand these checks Me neither. Since I don't know anything about Windows, I tried to keep the existing checks, but this doens't make much sense to me. Maybe Amaury can shed a light on this? -- Added file: http://bugs.python.org/file22326/is_selectable.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Changes by Charles-François Natali neolo...@free.fr: Added file: http://bugs.python.org/file22327/oss_check_closed.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Changes by Charles-François Natali neolo...@free.fr: Removed file: http://bugs.python.org/file22324/is_selectable.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Changes by Charles-François Natali neolo...@free.fr: Removed file: http://bugs.python.org/file22325/oss_check_closed.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Charles-François Natali neolo...@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 rep...@bugs.python.org
http://bugs.python.org/issue12287
___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,
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
STINNER Victor victor.stin...@haypocalc.com added the comment: 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? Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE is specific to Windows. /* WinSock does not use a bitmask in select, and uses socket handles greater than FD_SETSIZE */ #define Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE I don't understand if socket file descriptors are different than (classic) file descriptors. socketmodule.c uses Py_SAFE_DOWNCAST(s-sock_fd+1, SOCKET_T, int), which means that the socket type can be different (bigger) than int. #if defined(_MSC_VER) is maybe redundant with Py_SOCKET_FD_CAN_BE_GE_FD_SETSIZE (with #ifdef MS_WINDOWS ?). Does other compilers, like Cygwin / MinGW, also use WinSock? Where should I add it? selectmodule, posixmodule, somewhere else? _Py_Verify_fd() is defined in posixmodule.c and fileobject.h. If you would like write a _PyCheckSelectable_fd() function/macro, it can be defined in fileobject.h, and implemented anymore. selectmodule.c is maybe a better choice than posixmodule.c because posixmodule.c doesn't use select. Or maybe in socketmodule.c if you reuse IS_SELECTABLE code. For the name, I would prefer _PyIsSelectable_fd(). With check, I would have to read the name to check if it should return 0 or non-zero if the fd is selectable. -- Instead of _PyCheckSelectable_fd(), we can maybe do even better: write a function to check if a file descriptor is ready to read or write using poll() (of select() if poll() is not available). Example: int _Py_FDIsReady(int fd, int writing, double timeout); Returns: 1: fd is ready to read, or to write if writing is set 0: fd is not ready -1: error, check errno (or maybe raise a Python error? internal_select_ex() in socketmodule.c doesn't raise an exception) (_Py_FDIsReady name is horrible, but I don't have a better suggestion yet) poll() accepts negative timeout, whereas select() doesn't, and so _PyCheckSelectable_fd() should raise an error if timeout is negative to be portable. I propose to use poll() rather than select() because I suppose that it a little bit faster (maybe only if the fd number is big? e.g. fd=1023) The difference to wait a single file descriptor is maybe nul. What should be done in case of EINTR? PyThread_acquire_lock_timed() has an intr_flag parameter to decide. I don't think that int fd works with SOCKET_T (socket module), which can be bigger than an int. Well, _PyCheckSelectable_fd() is much more complex to implement than the initial suggestion... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
New submission from Charles-François Natali neolo...@free.fr: ossaudiodev's writeall method doesn't check that the FD is less than FD_SETSIZE when passing it to FD_SET: since FD_SET typically doesn't do bound check, it will write to a random location in memory (in this case on the stack). I've attached a test that triggers a segfault on my 32-bit Linux box: - you must have an OSS-compatible device as /dev/dsp (if you don't you can use modprobe snd_pcm_oss) - it tries to increase RLIMIT_NOFILE since it's usually defined to be the same as FD_SETSIZE (1024 on Linux). The script must be run as root for that. A patch is attached. The only other place where I've seen a similar problem is in Module/readline.c: I'm not sure it's worth adding this check there :-) -- components: Library (Lib) files: oss_select.diff keywords: needs review, patch messages: 137923 nosy: haypo, neologix, pitrou priority: normal severity: normal stage: patch review status: open title: ossaudiodev: stack corruption with FD = FD_SETSIZE type: crash versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3 Added file: http://bugs.python.org/file22284/oss_select.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Changes by Charles-François Natali neolo...@free.fr: Added file: http://bugs.python.org/file22285/test_oss.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
Antoine Pitrou pit...@free.fr added the comment: ossaudiodev's writeall method doesn't check that the FD is less than FD_SETSIZE when passing it to FD_SET: since FD_SET typically doesn't do bound check, it will write to a random location in memory (in this case on the stack). I've attached a test that triggers a segfault on my 32-bit Linux box: - you must have an OSS-compatible device as /dev/dsp (if you don't you can use modprobe snd_pcm_oss) - it tries to increase RLIMIT_NOFILE since it's usually defined to be the same as FD_SETSIZE (1024 on Linux). The script must be run as root for that. A patch is attached. Well, the test doesn't work here (IOError: [Errno 16] Device or resource busy: '/dev/dsp', probably because of PulseAudio already using it), but the patch looks simple enough. By the way, this function still uses y# instead of y*, this could be the topic of another issue if you are interested. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12287 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12287] ossaudiodev: stack corruption with FD = FD_SETSIZE
STINNER Victor victor.stin...@haypocalc.com added the comment:
You don't check that 0 = fd (e.g. oss.close()).
The select has a specific code for Visual Studio (don't check v FD_SETSIZE):
#if defined(_MSC_VER)
max = 0; /* not used for Win32 */
#else /* !_MSC_VER */
if (v 0 || v = FD_SETSIZE) {
PyErr_SetString(PyExc_ValueError,
filedescriptor out of range in select());
goto finally;
}
if (v max)
max = v;
#endif /* _MSC_VER */
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:
#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)
/* 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)
#endif
Note: do you really use the OSS module? On which OS? :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12287
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
