* fhandler_socket.cc (address_in_use): Disable. Add comment.

(fhandler_socket::bind): Change comment to explain setting the SO_EXCLUSIVEADDRUSE socket option. Remove code which checks for address in use. * net.cc (cygwin_setsockopt): Never set SO_REUSEADDR option. Improve comment to compensate for the deleted comment in fhandler_socket::bind. * wincap.cc: Throughout, drop has_enhanced_socket_security from wincaps. * wincap.h (struct wincaps): Drop has_enhanced_socket_security flags and method.
2011-01-30 21:52:12 +00:00 · 2011-01-30 21:52:12 +00:00 · a01454811a
parent 9ce2a54f70
commit a01454811a
5 changed files with 36 additions and 48 deletions
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@ -1,3 +1,15 @@
+2011-01-30  Corinna Vinschen  <corinna@vinschen.de>
+
+	* fhandler_socket.cc (address_in_use): Disable.  Add comment.
+	(fhandler_socket::bind): Change comment to explain setting the
+	SO_EXCLUSIVEADDRUSE socket option.  Remove code which checks for
+	address in use.
+	* net.cc (cygwin_setsockopt): Never set SO_REUSEADDR option.  Improve
+	comment to compensate for the deleted comment in fhandler_socket::bind.
+	* wincap.cc: Throughout, drop has_enhanced_socket_security from wincaps.
+	* wincap.h (struct wincaps): Drop has_enhanced_socket_security flags
+	and method.
+
 2011-01-28 Peter Foley <jpfoley2@verizon.net>

 	* configure.in: Define LIBSERVER regardless of cross_host.
--- a/winsup/cygwin/fhandler_socket.cc
+++ b/winsup/cygwin/fhandler_socket.cc
@ -872,6 +872,11 @@ fhandler_socket::link (const char *newpath)
  return fhandler_base::link (newpath);
 }

+#if 0
+/* This doesn't work correctly.  It has been called in bind to check if a
+   loca address is still in use, but it disables bind in the SO_REUSEADDR
+   case even if only an accepted socket is still using the local address.
+   I keep this function in the code for later reference only. */
 static inline bool
 address_in_use (const struct sockaddr *addr)
 {
@ -932,6 +937,7 @@ address_in_use (const struct sockaddr *addr)
    }
  return false;
 }
+#endif

 int
 fhandler_socket::bind (const struct sockaddr *name, int namelen)
@ -1039,10 +1045,10 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
    {
      if (!saw_reuseaddr ())
 	{
-	  /* If the application didn't explicitely call setsockopt
-	     (SO_REUSEADDR), enforce exclusive local address use using the
-	     SO_EXCLUSIVEADDRUSE socket option, to emulate POSIX socket
-	     behaviour more closely.
+	  /* If the application didn't explicitely request SO_REUSEADDR,
+	     enforce POSIX standard socket binding behaviour by setting the
+	     SO_EXCLUSIVEADDRUSE socket option.  See cygwin_setsockopt()
+	     for a more detailed description.

 	     KB 870562: Note that a bug in Win2K SP1-3 and XP up to SP1 only
 	     enables this option for users in the local administrators group. */
@ -1052,34 +1058,6 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
 				  (const char *) &on, sizeof on);
 	  debug_printf ("%d = setsockopt (SO_EXCLUSIVEADDRUSE), %E", ret);
 	}
-      else if (!wincap.has_enhanced_socket_security ())
-	{
-	  debug_printf ("SO_REUSEADDR set");
-	  /* There's a bug in SO_REUSEADDR handling in WinSock.
-	     Per standards, we must not be able to reuse a complete
-	     duplicate of a local TCP address (same IP, same port),
-	     even if SO_REUSEADDR has been set.  That's unfortunately
-	     possible in WinSock.
-
-	     So we're testing here if the local address is already in
-	     use and don't bind, if so.  This only works for OSes with
-	     IP Helper support and is, of course, still prone to races.
-
-	     However, we don't have to do this on systems supporting
-	     "enhanced socket security" (2K3 and later).  On these
-	     systems the default binding behaviour is exactly as you'd
-	     expect for SO_REUSEADDR, while setting SO_REUSEADDR re-enables
-	     the wrong behaviour.  So all we have to do on these newer
-	     systems is never to set SO_REUSEADDR but only to note that
-	     it has been set for the above SO_EXCLUSIVEADDRUSE setting.
-	     See setsockopt() in net.cc. */
-	  if (get_socket_type () == SOCK_STREAM && address_in_use (name))
-	    {
-	      debug_printf ("Local address in use, don't bind");
-	      set_errno (EADDRINUSE);
-	      goto out;
-	    }
-	}
      if (::bind (get_socket (), name, namelen))
 	set_winsock_errno ();
      else
--- a/winsup/cygwin/net.cc
+++ b/winsup/cygwin/net.cc
@ -706,12 +706,21 @@ cygwin_setsockopt (int fd, int level, int optname, const void *optval,
      if (level == IPPROTO_IP && CYGWIN_VERSION_CHECK_FOR_USING_WINSOCK1_VALUES)
 	optname = convert_ws1_ip_optname (optname);

-      /* On systems supporting "enhanced socket security (2K3 and later),
-	 the default behaviour of stream socket binding is equivalent to the
-	 POSIX behaviour with SO_REUSEADDR.  Setting SO_REUSEADDR would only
-	 result in wrong behaviour.  See also fhandler_socket::bind(). */
+      /* Per POSIX we must not be able to reuse a complete duplicate of a
+         local TCP address (same IP, same port), even if SO_REUSEADDR has been
+	 set.  That's unfortunately possible in WinSock, and this has never
+	 been changed to maintain backward compatibility.  Instead, the
+	 SO_EXCLUSIVEADDRUSE option has been added to allow an application to
+	 request POSIX standard behaviour in the non-SO_REUSEADDR case.
+
+	 However, the WinSock standard behaviour of stream socket binding
+	 is equivalent to the POSIX behaviour as if SO_REUSEADDR has been set.
+	 So what we do here is to note that SO_REUSEADDR has been set, but not
+	 actually hand over the request to WinSock.  This is tested in
+	 fhandler_socket::bind(), so that SO_EXCLUSIVEADDRUSE can be set if
+	 the application did not set SO_REUSEADDR.  This should reflect the
+	 POSIX socket binding behaviour as close as possible with WinSock. */
      if (level == SOL_SOCKET && optname == SO_REUSEADDR
-	  && wincap.has_enhanced_socket_security ()
 	  && fh->get_socket_type () == SOCK_STREAM)
 	res = 0;
      else
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@ -39,7 +39,6 @@ wincaps wincap_nt4sp4 __attribute__((section (".cygwin_dll_common"), shared)) =
  has_guid_volumes:false,
  has_disk_ex_ioctls:false,
  has_fileid_dirinfo:false,
-  has_enhanced_socket_security:false,
  has_buggy_restart_scan:false,
  has_mandatory_integrity_control:false,
  needs_logon_sid_in_sid_list:true,
@ -80,7 +79,6 @@ wincaps wincap_2000 __attribute__((section (".cygwin_dll_common"), shared)) = {
  has_guid_volumes:true,
  has_disk_ex_ioctls:false,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:false,
  has_buggy_restart_scan:true,
  has_mandatory_integrity_control:false,
  needs_logon_sid_in_sid_list:true,
@ -121,7 +119,6 @@ wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) =
  has_guid_volumes:true,
  has_disk_ex_ioctls:false,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:false,
  has_buggy_restart_scan:true,
  has_mandatory_integrity_control:false,
  needs_logon_sid_in_sid_list:true,
@ -162,7 +159,6 @@ wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = {
  has_guid_volumes:true,
  has_disk_ex_ioctls:true,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:false,
  has_buggy_restart_scan:false,
  has_mandatory_integrity_control:false,
  needs_logon_sid_in_sid_list:false,
@ -203,7 +199,6 @@ wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = {
  has_guid_volumes:true,
  has_disk_ex_ioctls:true,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:false,
  has_buggy_restart_scan:false,
  has_mandatory_integrity_control:false,
  needs_logon_sid_in_sid_list:false,
@ -244,7 +239,6 @@ wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = {
  has_guid_volumes:true,
  has_disk_ex_ioctls:true,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:false,
  has_buggy_restart_scan:false,
  has_mandatory_integrity_control:false,
  needs_logon_sid_in_sid_list:false,
@ -285,7 +279,6 @@ wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = {
  has_guid_volumes:true,
  has_disk_ex_ioctls:true,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:true,
  has_buggy_restart_scan:false,
  has_mandatory_integrity_control:false,
  needs_logon_sid_in_sid_list:false,
@ -326,7 +319,6 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
  has_guid_volumes:true,
  has_disk_ex_ioctls:true,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:true,
  has_buggy_restart_scan:false,
  has_mandatory_integrity_control:true,
  needs_logon_sid_in_sid_list:false,
@ -367,7 +359,6 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
  has_guid_volumes:true,
  has_disk_ex_ioctls:true,
  has_fileid_dirinfo:true,
-  has_enhanced_socket_security:true,
  has_buggy_restart_scan:false,
  has_mandatory_integrity_control:true,
  needs_logon_sid_in_sid_list:false,
--- a/winsup/cygwin/wincap.h
+++ b/winsup/cygwin/wincap.h
@ -29,7 +29,6 @@ struct wincaps
  unsigned has_guid_volumes				: 1;
  unsigned has_disk_ex_ioctls				: 1;
  unsigned has_fileid_dirinfo				: 1;
-  unsigned has_enhanced_socket_security			: 1;
  unsigned has_buggy_restart_scan			: 1;
  unsigned has_mandatory_integrity_control		: 1;
  unsigned needs_logon_sid_in_sid_list			: 1;
@ -86,7 +85,6 @@ public:
  bool	IMPLEMENT (has_guid_volumes)
  bool	IMPLEMENT (has_disk_ex_ioctls)
  bool	IMPLEMENT (has_fileid_dirinfo)
-  bool	IMPLEMENT (has_enhanced_socket_security)
  bool	IMPLEMENT (has_buggy_restart_scan)
  bool	IMPLEMENT (has_mandatory_integrity_control)
  bool	IMPLEMENT (needs_logon_sid_in_sid_list)