[Python-checkins] r42254 - in python/branches/release24-maint: Misc/NEWS Modules/_ssl.c Modules/socketmodule.c

Jim Jewett jimjjewett at gmail.com
Tue Feb 7 16:36:52 CET 2006


Is there any other way that a file descriptor could be invalid?

I keep wanting to see SOCKET_TOO_LARGE instead of
SOCKET_INVALID since that is all it checks.

Also, if I am understanding correctly, the problem isn't
with the size of the socket, it is with the total number of
file descriptors, and this socket just happened to get one
outside the valid set (which is presumably numbered
sequentially).

Maybe SOCKET_INVALID_FD ?

(Also, would a negative number or float or something cause the
same problems, except that they presumably get weeded out
earlier?)

-jJ

On 2/7/06, neal.norwitz <python-checkins at python.org> wrote:
> Author: neal.norwitz
> Date: Tue Feb  7 08:17:37 2006
> New Revision: 42254
>
> Modified:
>    python/branches/release24-maint/Misc/NEWS
>    python/branches/release24-maint/Modules/_ssl.c
>    python/branches/release24-maint/Modules/socketmodule.c
> Log:
> Backport:
> Bug #876637, prevent stack corruption when socket descriptor
> is larger than FD_SETSIZE.
>
> This can only be acheived with ulimit -n SOME_NUMBER_BIGGER_THAN_FD_SETSIZE
> which is typically only available to root.  Since this wouldn't normally
> be run in a test (ie, run as root), it doesn't seem too worthwhile to
> add a normal test.  The bug report has one version of a test.  I've
> written another.  Not sure what the best thing to do is.
>
> Do the check before calling internal_select() because we can't set
> an error in between Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS.
> This seemed the clearest solution.
>
>
>
> Modified: python/branches/release24-maint/Misc/NEWS
> ==============================================================================
> --- python/branches/release24-maint/Misc/NEWS   (original)
> +++ python/branches/release24-maint/Misc/NEWS   Tue Feb  7 08:17:37 2006
> @@ -45,6 +45,9 @@
>  Extension Modules
>  -----------------
>
> +- Bug #876637, prevent stack corruption when socket descriptor
> +  is larger than FD_SETSIZE.
> +
>  - Patch #1407135, bug #1424041: mmap.mmap(-1, size, ...) can return
>    anonymous memory again on Unix.
>
>
> Modified: python/branches/release24-maint/Modules/_ssl.c
> ==============================================================================
> --- python/branches/release24-maint/Modules/_ssl.c      (original)
> +++ python/branches/release24-maint/Modules/_ssl.c      Tue Feb  7 08:17:37 2006
> @@ -74,6 +74,7 @@
>         SOCKET_IS_BLOCKING,
>         SOCKET_HAS_TIMED_OUT,
>         SOCKET_HAS_BEEN_CLOSED,
> +       SOCKET_INVALID,
>         SOCKET_OPERATION_OK
>  } timeout_state;
>
> @@ -272,6 +273,9 @@
>                 } else if (sockstate == SOCKET_HAS_BEEN_CLOSED) {
>                         PyErr_SetString(PySSLErrorObject, "Underlying socket has been closed.");
>                         goto fail;
> +               } else if (sockstate == SOCKET_INVALID) {
> +                       PyErr_SetString(PySSLErrorObject, "Underlying socket too large for select().");
> +                       goto fail;
>                 } else if (sockstate == SOCKET_IS_NONBLOCKING) {
>                         break;
>                 }
> @@ -372,6 +376,10 @@
>         if (s->sock_fd < 0)
>                 return SOCKET_HAS_BEEN_CLOSED;
>
> +       /* Guard against socket too large for select*/
> +       if (s->sock_fd >= FD_SETSIZE)
> +               return SOCKET_INVALID;
> +
>         /* Construct the arguments to select */
>         tv.tv_sec = (int)s->sock_timeout;
>         tv.tv_usec = (int)((s->sock_timeout - tv.tv_sec) * 1e6);
> @@ -409,6 +417,9 @@
>         } else if (sockstate == SOCKET_HAS_BEEN_CLOSED) {
>                 PyErr_SetString(PySSLErrorObject, "Underlying socket has been closed.");
>                 return NULL;
> +       } else if (sockstate == SOCKET_INVALID) {
> +               PyErr_SetString(PySSLErrorObject, "Underlying socket too large for select().");
> +               return NULL;
>         }
>         do {
>                 err = 0;
> @@ -467,6 +478,9 @@
>                 PyErr_SetString(PySSLErrorObject, "The read operation timed out");
>                 Py_DECREF(buf);
>                 return NULL;
> +       } else if (sockstate == SOCKET_INVALID) {
> +               PyErr_SetString(PySSLErrorObject, "Underlying socket too large for select().");
> +               return NULL;
>         }
>         do {
>                 err = 0;
>
> Modified: python/branches/release24-maint/Modules/socketmodule.c
> ==============================================================================
> --- python/branches/release24-maint/Modules/socketmodule.c      (original)
> +++ python/branches/release24-maint/Modules/socketmodule.c      Tue Feb  7 08:17:37 2006
> @@ -390,6 +390,16 @@
>     there has to be a circular reference. */
>  static PyTypeObject sock_type;
>
> +/* Can we call select() with this socket without a buffer overrun? */
> +#define IS_SELECTABLE(s) ((s)->sock_fd < FD_SETSIZE)
> +
> +static PyObject*
> +select_error(void)
> +{
> +       PyErr_SetString(socket_error, "unable to select on socket");
> +       return NULL;
> +}
> +
>  /* Convenience function to raise an error according to errno
>     and return a NULL pointer from a function. */
>
> @@ -1362,6 +1372,9 @@
>         newfd = -1;
>  #endif
>
> +       if (!IS_SELECTABLE(s))
> +               return select_error();
> +
>         Py_BEGIN_ALLOW_THREADS
>         timeout = internal_select(s, 0);
>         if (!timeout)
> @@ -1690,7 +1703,8 @@
>  #ifdef MS_WINDOWS
>
>         if (s->sock_timeout > 0.0) {
> -               if (res < 0 && WSAGetLastError() == WSAEWOULDBLOCK) {
> +               if (res < 0 && WSAGetLastError() == WSAEWOULDBLOCK &&
> +                   IS_SELECTABLE(s)) {
>                         /* This is a mess.  Best solution: trust select */
>                         fd_set fds;
>                         fd_set fds_exc;
> @@ -1735,7 +1749,7 @@
>  #else
>
>         if (s->sock_timeout > 0.0) {
> -               if (res < 0 && errno == EINPROGRESS) {
> +               if (res < 0 && errno == EINPROGRESS && IS_SELECTABLE(s)) {
>                         timeout = internal_select(s, 1);
>                         res = connect(s->sock_fd, addr, addrlen);
>                         if (res < 0 && errno == EISCONN)
> @@ -2038,6 +2052,9 @@
>         if (buf == NULL)
>                 return NULL;
>
> +       if (!IS_SELECTABLE(s))
> +               return select_error();
> +
>  #ifndef __VMS
>         Py_BEGIN_ALLOW_THREADS
>         timeout = internal_select(s, 0);
> @@ -2131,6 +2148,9 @@
>         if (buf == NULL)
>                 return NULL;
>
> +       if (!IS_SELECTABLE(s))
> +               return select_error();
> +
>         Py_BEGIN_ALLOW_THREADS
>         memset(&addrbuf, 0, addrlen);
>         timeout = internal_select(s, 0);
> @@ -2192,6 +2212,9 @@
>         if (!PyArg_ParseTuple(args, "s#|i:send", &buf, &len, &flags))
>                 return NULL;
>
> +       if (!IS_SELECTABLE(s))
> +               return select_error();
> +
>  #ifndef __VMS
>         Py_BEGIN_ALLOW_THREADS
>         timeout = internal_select(s, 1);
> @@ -2257,6 +2280,9 @@
>         if (!PyArg_ParseTuple(args, "s#|i:sendall", &buf, &len, &flags))
>                 return NULL;
>
> +       if (!IS_SELECTABLE(s))
> +               return select_error();
> +
>         Py_BEGIN_ALLOW_THREADS
>         do {
>                 timeout = internal_select(s, 1);
> @@ -2311,6 +2337,9 @@
>         if (!getsockaddrarg(s, addro, &addr, &addrlen))
>                 return NULL;
>
> +       if (!IS_SELECTABLE(s))
> +               return select_error();
> +
>         Py_BEGIN_ALLOW_THREADS
>         timeout = internal_select(s, 1);
>         if (!timeout)
> _______________________________________________
> Python-checkins mailing list
> Python-checkins at python.org
> http://mail.python.org/mailman/listinfo/python-checkins
>


More information about the Python-checkins mailing list