From:	noer AT cygnus DOT com (Geoffrey Noer)
Subject:	Re: select() bug and fix
20 Apr 1998 15:24:31 -0700 :
Message-ID:	<199804202209.PAA09511.cygnus.cygwin32.developers@skaro.cygnus.com>
References:	<m0yPXgu-00113YC AT malasada DOT lava DOT net>
To:	newsham AT lava DOT net (Tim Newsham)
Cc:	cygwin32-developers AT cygnus DOT com

I committed this change.

-- 
Geoffrey Noer
noer AT cygnus DOT com

Tim Newsham wrote:
> 
>      There is a bug in select() when you select on sockets for exceptions.
> The bug causes memory corruption which causes the allocator to finally
> crash while doing free()'s.  The bug is due to an off-by-one error in
> calculating the winsock descriptor set for exceptions -- The code counts
> the number of sockets in the except descriptor set and allocates a
> winsock descriptor set of that size.  The select thread then goes and
> adds one more descriptor to that set, overrunning the end of the winsock
> descriptor set's buffer.
> 
> The fix is simple enough.  In select.cc:cygwin32_select()
> 
> (note: my patch line numbers are likely off,  I'm using b19.1 sources
> with some of my own patches for other things)
> ---- start patch snip ----
> @@ -767,7 +730,7 @@
> 
>    read_socket_map = new fd_socket_map (total_read_number);
>    write_socket_map = new fd_socket_map (total_write_number);
> -  except_socket_map = new fd_socket_map (total_except_number);
> +  except_socket_map = new fd_socket_map (total_except_number + 1);
> 
>    fd_read_console_map *read_console_map = new fd_read_console_map (total_read_number);
>    read_pipe_map = new fd_pipe_map (total_read_number);
> ---- end patch snip ----
> 
> The select code has a few other problems:
> 
>     - memcpy() is used to copy memory over itself (is the memcpy in the
>       library capable of this?).  At any rate the code can be made simpler
>       and more efficient:
> 
> ---- start patch snip ----
> @@ -197,14 +163,12 @@
>        {
>          if (i < (used_ - 1))
>            {
> -            memcpy ((char *) &handle_array_[i],
> -                   (char *) &handle_array_[i + 1],
> -                   (used_ - i - 1) * sizeof (HANDLE));
> -            memcpy ((char *) &unix_fd_array_[i],
> -                   (char *) &unix_fd_array_[i + 1],
> -                   (used_ - i - 1) * sizeof (unsigned short));
> +            /* move last entry to new empty slot */
> +            handle_array_[i] = handle_array_[used_ - 1];
> +            unix_fd_array_[i] = unix_fd_array_[used_ - 1];
>            }
>          --used_;
> +        break;
>        }
>  }
> ---- end patch snip ----
> 
> 
>     - select assumes in a number of places that the fd sets have only
>       64 bits in them.  This means that you can only use 64 descriptors
>       in programs that select (even though cygwin allows you to open
>       many more descriptors than this).  I haven't patched this yet.
> 
>                                              Tim N.

- Raw text -