Discussion:
[newlib-cygwin] Cygwin: fix memory corruption/SEGV if certain socket functions fail
Corinna Vinschen
2018-10-29 15:35:00 UTC
Permalink
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=2bbe8697d8f14eca88d8d45c11a5a58e879a3c0f

commit 2bbe8697d8f14eca88d8d45c11a5a58e879a3c0f
Author: Corinna Vinschen <***@vinschen.de>
Date: Mon Oct 29 16:12:54 2018 +0100

Cygwin: fix memory corruption/SEGV if certain socket functions fail

Regression introduced with 2.11.0:

The failure paths in socket, socketpair and accept4 functions and
methods accidentally release *unused* cygheap_fdmanip objects. The
subsequently called dtable::release method was designed to be called for
*used* cygheap_fdmanip objects only. Using them on unused objects leads
to NULL pointer member dereferencing.

Worse, the inet/local accept4 methods only release the cygheap_fdmanip
object but neglect to delete the just created fhandler_socket_* object.

Fix this by removing the erroneous release calls in the aforementioned
failure paths and delete the fhandler_socket_* object in accept4 instead.

Signed-off-by: Corinna Vinschen <***@vinschen.de>

Diff:
---
winsup/cygwin/fhandler_socket_inet.cc | 2 +-
winsup/cygwin/fhandler_socket_local.cc | 3 +--
winsup/cygwin/fhandler_socket_unix.cc | 1 -
winsup/cygwin/net.cc | 12 ++----------
winsup/cygwin/release/2.11.2 | 4 ++++
5 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/winsup/cygwin/fhandler_socket_inet.cc b/winsup/cygwin/fhandler_socket_inet.cc
index 555c3a9..63abf73 100644
--- a/winsup/cygwin/fhandler_socket_inet.cc
+++ b/winsup/cygwin/fhandler_socket_inet.cc
@@ -898,7 +898,7 @@ fhandler_socket_inet::accept4 (struct sockaddr *peer, int *len, int flags)
}
}
else
- fd.release ();
+ delete sock;
}
if (ret == -1)
::closesocket (res);
diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc
index bffb112..f88ced2 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -988,7 +988,6 @@ fhandler_socket_local::accept4 (struct sockaddr *peer, int *len, int flags)
ret = sock->af_local_accept ();
if (ret == -1)
{
- fd.release ();
delete sock;
set_winsock_errno ();
return -1;
@@ -1013,7 +1012,7 @@ fhandler_socket_local::accept4 (struct sockaddr *peer, int *len, int flags)
}
}
else
- fd.release ();
+ delete sock;
}
if (ret == -1)
::closesocket (res);
diff --git a/winsup/cygwin/fhandler_socket_unix.cc b/winsup/cygwin/fhandler_socket_unix.cc
index 787d2be..da83ecc 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -1639,7 +1639,6 @@ fhandler_socket_unix::accept4 (struct sockaddr *peer, int *len, int flags)
create_shmem_failed:
delete sock;
}
- fd.release ();
}
}
/* Ouch! We can't handle the client if we couldn't
diff --git a/winsup/cygwin/net.cc b/winsup/cygwin/net.cc
index d152894..4494bf7 100644
--- a/winsup/cygwin/net.cc
+++ b/winsup/cygwin/net.cc
@@ -536,10 +536,7 @@ cygwin_socket (int af, int type, int protocol)
res = fd;
}
else
- {
- delete fh;
- fd.release ();
- }
+ delete fh;
}

done:
@@ -2314,10 +2311,7 @@ socketpair (int af, int type, int protocol, int sv[2])

cygheap_fdnew fd_out (fd_in, false);
if (fd_out < 0)
- {
- fd_in.release ();
- goto done;
- }
+ goto done;

fh_in = reinterpret_cast<fhandler_socket *> (build_fh_dev (*dev));
fh_out = reinterpret_cast<fhandler_socket *> (build_fh_dev (*dev));
@@ -2343,8 +2337,6 @@ socketpair (int af, int type, int protocol, int sv[2])
{
delete fh_in;
delete fh_out;
- fd_in.release ();
- fd_out.release ();
}
}

diff --git a/winsup/cygwin/release/2.11.2 b/winsup/cygwin/release/2.11.2
index b9ccc5c..2ae5fbc 100644
--- a/winsup/cygwin/release/2.11.2
+++ b/winsup/cygwin/release/2.11.2
@@ -14,3 +14,7 @@ Bug Fixes

- Fix a memory corruption when using pipes or FIFOs
Addresses: https://cygwin.com/ml/cygwin-patches/2018-q4/msg00000.html
+
+- Fix potential memory corruption and SEGV if socket(2), socketpair(2),
+ accept(2), or accept4(2) fail.
+ Addresses: https://cygwin.com/ml/cygwin/2018-10/msg00229.html

Loading...