On Thu, Mar 3, 2016 at 8:48 AM, James Yonan <ja...@openvpn.net> wrote:
> Fixed port-share bug that can cause segfault when the number
> of concurrent connections is large.
>
> The issue is that the port-share code calls openvpn_connect()
> which in turn calls select(). When there are a high number
> of concurrent port-share connections, the fd passed to select
> can potentially exceed FD_SETSIZE, causing undefined behavior.
>
> The fix is to use poll() (if available) instead of select().
>
> Signed-off-by: James Yonan <ja...@openvpn.net>
> ---
> src/openvpn/socket.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
> index 714a847..d33bccd 100644
> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -1149,6 +1149,12 @@ openvpn_connect (socket_descriptor_t sd,
> {
> while (true)
> {
> +#if POLL
> + struct pollfd fds[1];
> + fds[0].fd = sd;
> + fds[0].events = POLLOUT;
> + status = poll(fds, 1, 0);
> +#else
> fd_set writes;
> struct timeval tv;
>
> @@ -1158,7 +1164,7 @@ openvpn_connect (socket_descriptor_t sd,
> tv.tv_usec = 0;
>
> status = select (sd + 1, NULL, &writes, NULL, &tv);
> -
> +#endif
> if (signal_received)
> {
> get_signal (signal_received);
ACK
Note that since patch e0b3fd49 in master and 1746908f in release/2.3
this no longer causes a segfault, but triggers an ASSERT().
In the long run, we should migrate away from select() completely.
-Steffan
(This patch was first discussed and reviewed on the closed security@ list)
