Re: Problems of AF_INET domain socket regarding out-of-band data.

[Takashi Yano <takashi.yano@nifty.ne.jp>]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

Hi Corinna,

On Wed, 13 Jun 2018 22:48:58 +0900
Takashi Yano wrote:
> 1. recv() with MSG_OOB flag eats normal data if no OOB data
>   is sent yet.
> 
> 2. Calling recv() with MSG_OOB flag is blocked if no OOB data
>   is sent yet.
> 
> 3. Calling recv() without MSG_OOB flag after receiving OOB data
>   is blocked even if received data exist in buffer. 

I looked into these problems and found these are due to bug of
cygwin1.dll.

Problem 1:
If recv() is called with MSG_OOB, in fhandler_socket_inet::
recv_internal(), wsamsg->dwFlags with MSG_OOB flag set is passed
to WSARecv() and this fails because no OOB data exists. At this time,
wsamsg-> dwFlags is modified by WSARecv() so that it does not have
the MSG_OOB flag. Then, WSARecv() is called again without MSG_OOB
flag in while loop. At this time, normal data is read and returned.

Problem 2:
In fhandler_socket_inet::recv_internal(), wait_for_events() is
called. This blocks the call until OOB data arrives.

Problem 3:
If recv() is called with MSG_OOB flag set, fhandler_socket_inet::
recv_internal() calls wait_for_events() with both FD_OOB and
FD_READ. If both OOB data and normal data already arrived,
not only the event of FD_OOB but also the event of FD_READ are
reset to non signaled state. I'm not sure where the signal is
reset, though.

Moreover, return value of ioctl command SIOCATMARK of winsock
is not as expected. In winsock, SIOCATMARK returns TRUE if no
OOB data exists, FALSE otherwise. This is almost opposite to
expectation.

Furthermore, inline mode (SO_OOBINLINE) of winsock is completely
broken. If SO_OOBINLINE is set, SIOATMARK always returns TRUE.
This means application cannot determine OOB data at all in inline
mode.

To solve these problems, I made a patch attached.

Could you please have a look?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-Fix-the-handling-of-out-band-data-OOB-in-a-socket.patch --]
[-- Type: application/octet-stream, Size: 13017 bytes --]

From 63497ad1d5089beb21a63c62de3c662faf42881f Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Wed, 20 Jun 2018 14:47:27 +0900
Subject: [PATCH] Fix the handling of out-band-data (OOB) in a socket.

* fhandler.h (class fhandler_socket_wsock): Add variable bool oobinline.
* fhandler_socket_inet.cc (fhandler_socket_wsock::fhandler_socket_wsock):
  Add inisialize of oobinline.
(fhandler_socket_inet::recv_internal): Make the handling of OOB data
  as consistent with POSIX as possible. Add simulation of inline mode
  for OOB data as a workaround for broken winsock behaviour.
(fhandler_socket_inet::setsockopt): Ditto.
(fhandler_socket_inet::getsockopt): Ditto.
(fhandler_socket_wsock::ioctl): Fix return value of SIOCATMARK command.
  The return value of SIOCATMARK of winsock is almost opposite to
  expectation.
* fhandler_socket_local.cc (fhandler_socket_local::recv_internal):
  Make the handling of OOB data as consistent with POSIX as possible.
  Add simulation of inline mode for OOB data as a workaround for broken
  winsock behaviour.
(fhandler_socket_local::setsockopt): Ditto.
(fhandler_socket_local::getsockopt): Ditto.

This fixes the issue reported in following post.
https://cygwin.com/ml/cygwin/2018-06/msg00143.html
---
 winsup/cygwin/fhandler.h               |  1 +
 winsup/cygwin/fhandler_socket_inet.cc  | 92 +++++++++++++++++++++++++-
 winsup/cygwin/fhandler_socket_local.cc | 80 +++++++++++++++++++++-
 3 files changed, 167 insertions(+), 6 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 2ec460a37..8222fb835 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -601,6 +601,7 @@ class fhandler_socket_wsock: public fhandler_socket
   wsa_event *wsock_events;
   HANDLE wsock_mtx;
   HANDLE wsock_evt;
+  bool oobinline; /* True if option SO_OOBINLINE is set */
   bool init_events ();
   int wait_for_events (const long event_mask, const DWORD flags);
   void release_events ();
diff --git a/winsup/cygwin/fhandler_socket_inet.cc b/winsup/cygwin/fhandler_socket_inet.cc
index db301f354..24baf9c89 100644
--- a/winsup/cygwin/fhandler_socket_inet.cc
+++ b/winsup/cygwin/fhandler_socket_inet.cc
@@ -201,6 +201,7 @@ fhandler_socket_wsock::fhandler_socket_wsock () :
   wsock_events (NULL),
   wsock_mtx (NULL),
   wsock_evt (NULL),
+  oobinline (false),
   status (),
   prot_info_ptr (NULL)
 {
@@ -1044,10 +1045,11 @@ fhandler_socket_inet::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
 {
   ssize_t res = 0;
   DWORD ret = 0, wret;
-  int evt_mask = FD_READ | ((wsamsg->dwFlags & MSG_OOB) ? FD_OOB : 0);
+  int evt_mask = (wsamsg->dwFlags & MSG_OOB) ? FD_OOB : FD_READ;
   LPWSABUF &wsabuf = wsamsg->lpBuffers;
   ULONG &wsacnt = wsamsg->dwBufferCount;
   static NO_COPY LPFN_WSARECVMSG WSARecvMsg;
+  bool read_oob = false;
 
   /* CV 2014-10-26: Do not check for the connect_state at this point.  In
      certain scenarios there's no way to check the connect state reliably.
@@ -1086,12 +1088,64 @@ fhandler_socket_inet::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
 	waitall = false;
     }
 
+  /* recv() returns EINVAL if MSG_OOB flag is set in inline mode. */
+  if (oobinline && (wsamsg->dwFlags & MSG_OOB))
+    {
+      set_errno (EINVAL);
+      return SOCKET_ERROR;
+    }
+
+  /* Check whether OOB data is ready or not */
+  if (get_socket_type () == SOCK_STREAM)
+    if ((wsamsg->dwFlags & MSG_OOB) || oobinline)
+      {
+	u_long atmark = 0;
+#ifdef __x86_64__
+	/* SIOCATMARK = _IOR('s',7,u_long) */
+	int err = ::ioctlsocket (get_socket (), _IOR('s',7,u_long), &atmark);
+#else
+	int err = ::ioctlsocket (get_socket (), SIOCATMARK, &atmark);
+#endif
+	if (err)
+	  {
+	    set_winsock_errno ();
+	    return SOCKET_ERROR;
+	  }
+	/* If there is no OOB data, recv() with MSG_OOB returns EINVAL.
+	   Note: The return value of SIOCATMARK in non-inline mode of
+	   winsock is FALSE if OOB data exists, TRUE otherwise. */
+	if (atmark && (wsamsg->dwFlags & MSG_OOB))
+	  {
+	    /* No OOB data */
+	    set_errno (EINVAL);
+	    return SOCKET_ERROR;
+	  }
+	/* Inline mode for oub-of-band (OOB) data of winsock is
+	   completely broken. That is, SIOCATMARK always returns
+	   TRUE in inline mode. Due to this problem, application
+	   cannot determine OOB data at all. Therefore the behavior
+	   of a socket with SO_OOBINLINE set is simulated using
+	   a socket with SO_OOBINLINE not set. In this fake inline
+	   mode, the order of the OOB and non-OOB data is not
+	   preserved. OOB data is read before non-OOB data sent
+	   prior to the OOB data.  However, this is most likely
+	   not a problem in most cases. */
+	/* If there is OOB data, read OOB data using MSG_OOB in
+	   fake inline mode. */
+	if (!atmark && oobinline)
+	  {
+	    read_oob = true;
+	    evt_mask = FD_OOB;
+	  }
+      }
+
   /* Note: Don't call WSARecvFrom(MSG_PEEK) without actually having data
      waiting in the buffers, otherwise the event handling gets messed up
      for some reason. */
   while (!(res = wait_for_events (evt_mask | FD_CLOSE, wait_flags))
 	 || saw_shutdown_read ())
     {
+      DWORD dwFlags = wsamsg->dwFlags | (read_oob ? MSG_OOB : 0);
       if (use_recvmsg)
 	res = WSARecvMsg (get_socket (), wsamsg, &wret, NULL, NULL);
       /* This is working around a really weird problem in WinSock.
@@ -1113,11 +1167,11 @@ fhandler_socket_inet::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
 	 namelen is a valid pointer while name is NULL.  Both parameters are
 	 ignored for TCP sockets, so this only occurs when using UDP socket. */
       else if (!wsamsg->name || get_socket_type () == SOCK_STREAM)
-	res = WSARecv (get_socket (), wsabuf, wsacnt, &wret, &wsamsg->dwFlags,
+	res = WSARecv (get_socket (), wsabuf, wsacnt, &wret, &dwFlags,
 		       NULL, NULL);
       else
 	res = WSARecvFrom (get_socket (), wsabuf, wsacnt, &wret,
-			   &wsamsg->dwFlags, wsamsg->name, &wsamsg->namelen,
+			   &dwFlags, wsamsg->name, &wsamsg->namelen,
 			   NULL, NULL);
       if (!res)
 	{
@@ -1561,6 +1615,23 @@ fhandler_socket_inet::setsockopt (int level, int optname, const void *optval,
 	    set_errno (EDOM);
 	  return ret;
 
+	case SO_OOBINLINE:
+	  /* Inline mode for oub-of-band (OOB) data of winsock is
+	     completely broken. That is, SIOCATMARK always returns
+	     TRUE in inline mode. Due to this problem, application
+	     cannot determine OOB data at all. Therefore the behavior
+	     of a socket with SO_OOBINLINE set is simulated using
+	     a socket with SO_OOBINLINE not set. In this fake inline
+	     mode, the order of the OOB and non-OOB data is not
+	     preserved. OOB data is read before non-OOB data sent
+	     prior to the OOB data.  However, this is most likely
+	     not a problem in most cases. */
+	  /* Here, instead of actually setting inline mode, simply
+	     set the variable oobinline. */
+	  oobinline = *(int *) optval ? true : false;
+	  ignore = true;
+	  break;
+
 	default:
 	  break;
 	}
@@ -1713,6 +1784,10 @@ fhandler_socket_inet::getsockopt (int level, int optname, const void *optval,
 	    return 0;
 	  }
 
+	case SO_OOBINLINE:
+	  *(int *) optval = oobinline ? 1 : 0;
+	  return 0;
+
 	default:
 	  break;
 	}
@@ -1850,6 +1925,17 @@ fhandler_socket_wsock::ioctl (unsigned int cmd, void *p)
 	}
       else
 	res = ::ioctlsocket (get_socket (), cmd, (u_long *) p);
+      /* In winsock, the return value of SIOCATMARK is FALSE if
+	 OOB data exists, TRUE otherwise. This is almost opposite
+	 to expectation. */
+#ifdef __x86_64__
+      /* SIOCATMARK = _IOR('s',7,u_long) */
+      if (cmd == _IOR('s',7,u_long) && !res)
+	*(u_long *)p = !*(u_long *)p;
+#else
+      if (cmd == SIOCATMARK && !res)
+	*(u_long *)p = !*(u_long *)p;
+#endif
       break;
     default:
       res = fhandler_socket::ioctl (cmd, p);
diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc
index 90d8d5aa5..7f776167a 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -1065,11 +1065,12 @@ fhandler_socket_local::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
 {
   ssize_t res = 0;
   DWORD ret = 0, wret;
-  int evt_mask = FD_READ | ((wsamsg->dwFlags & MSG_OOB) ? FD_OOB : 0);
+  int evt_mask = (wsamsg->dwFlags & MSG_OOB) ? FD_OOB : FD_READ;
   LPWSABUF &wsabuf = wsamsg->lpBuffers;
   ULONG &wsacnt = wsamsg->dwBufferCount;
   static NO_COPY LPFN_WSARECVMSG WSARecvMsg;
   int orig_namelen = wsamsg->namelen;
+  bool read_oob = false;
 
   /* CV 2014-10-26: Do not check for the connect_state at this point.  In
      certain scenarios there's no way to check the connect state reliably.
@@ -1108,12 +1109,65 @@ fhandler_socket_local::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
 	waitall = false;
     }
 
+  /* recv() returns EINVAL if MSG_OOB flag is set in inline mode. */
+  if (oobinline && (wsamsg->dwFlags & MSG_OOB))
+    {
+      set_errno (EINVAL);
+      return SOCKET_ERROR;
+    }
+
+  /* Check whether OOB data is ready or not */
+  if (get_socket_type () == SOCK_STREAM)
+    if ((wsamsg->dwFlags & MSG_OOB) || oobinline)
+      {
+	u_long atmark = 0;
+	/* SIOCATMARK = _IOR('s',7,u_long) */
+#ifdef __x86_64__
+	/* SIOCATMARK = _IOR('s',7,u_long) */
+	int err = ::ioctlsocket (get_socket (), _IOR('s',7,u_long), &atmark);
+#else
+	int err = ::ioctlsocket (get_socket (), SIOCATMARK, &atmark);
+#endif
+	if (err)
+	  {
+	    set_winsock_errno ();
+	    return SOCKET_ERROR;
+	  }
+	/* If there is no OOB data, recv() with MSG_OOB returns EINVAL.
+	   Note: The return value of SIOCATMARK in non-inline mode of
+	   winsock is FALSE if OOB data exists, TRUE otherwise. */
+	if (atmark && (wsamsg->dwFlags & MSG_OOB))
+	  {
+	    /* No OOB data */
+	    set_errno (EINVAL);
+	    return SOCKET_ERROR;
+	  }
+	/* Inline mode for oub-of-band (OOB) data of winsock is
+	   completely broken. That is, SIOCATMARK always returns
+	   TRUE in inline mode. Due to this problem, application
+	   cannot determine OOB data at all. Therefore the behavior
+	   of a socket with SO_OOBINLINE set is simulated using
+	   a socket with SO_OOBINLINE not set. In this fake inline
+	   mode, the order of the OOB and non-OOB data is not
+	   preserved. OOB data is read before non-OOB data sent
+	   prior to the OOB data.  However, this is most likely
+	   not a problem in most cases. */
+	/* If there is OOB data, read OOB data using MSG_OOB in
+	   fake inline mode. */
+	if (!atmark && oobinline)
+	  {
+	    read_oob = true;
+	    evt_mask = FD_OOB;
+	  }
+      }
+
   /* Note: Don't call WSARecvFrom(MSG_PEEK) without actually having data
      waiting in the buffers, otherwise the event handling gets messed up
      for some reason. */
   while (!(res = wait_for_events (evt_mask | FD_CLOSE, wait_flags))
 	 || saw_shutdown_read ())
     {
+      DWORD dwFlags = wsamsg->dwFlags | (read_oob ? MSG_OOB : 0);
       if (use_recvmsg)
 	res = WSARecvMsg (get_socket (), wsamsg, &wret, NULL, NULL);
       /* This is working around a really weird problem in WinSock.
@@ -1135,11 +1189,11 @@ fhandler_socket_local::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
 	 namelen is a valid pointer while name is NULL.  Both parameters are
 	 ignored for TCP sockets, so this only occurs when using UDP socket. */
       else if (!wsamsg->name || get_socket_type () == SOCK_STREAM)
-	res = WSARecv (get_socket (), wsabuf, wsacnt, &wret, &wsamsg->dwFlags,
+	res = WSARecv (get_socket (), wsabuf, wsacnt, &wret, &dwFlags,
 		       NULL, NULL);
       else
 	res = WSARecvFrom (get_socket (), wsabuf, wsacnt, &wret,
-			   &wsamsg->dwFlags, wsamsg->name, &wsamsg->namelen,
+			   &dwFlags, wsamsg->name, &wsamsg->namelen,
 			   NULL, NULL);
       if (!res)
 	{
@@ -1396,6 +1450,22 @@ fhandler_socket_local::setsockopt (int level, int optname, const void *optval,
 	case SO_SNDLOWAT:
 	  break;
 
+	case SO_OOBINLINE:
+	  /* Inline mode for oub-of-band (OOB) data of winsock is
+	     completely broken. That is, SIOCATMARK always returns
+	     TRUE in inline mode. Due to this problem, application
+	     cannot determine OOB data at all. Therefore the behavior
+	     of a socket with SO_OOBINLINE set is simulated using
+	     a socket with SO_OOBINLINE not set. In this fake inline
+	     mode, the order of the OOB and non-OOB data is not
+	     preserved. OOB data is read before non-OOB data sent
+	     prior to the OOB data.  However, this is most likely
+	     not a problem in most cases. */
+	  /* Here, instead of actually setting inline mode, simply
+	     set the variable oobinline. */
+	  oobinline = *(int *) optval ? true : false;
+	  return 0;
+
 	default:
 	  /* AF_LOCAL sockets simply ignore all other SOL_SOCKET options. */
 	  return 0;
@@ -1538,6 +1608,10 @@ fhandler_socket_local::getsockopt (int level, int optname, const void *optval,
 	    return 0;
 	  }
 
+	case SO_OOBINLINE:
+	  *(int *) optval = oobinline ? 1 : 0;
+	  return 0;
+
 	default:
 	  {
 	    unsigned int *val = (unsigned int *) optval;
-- 
2.17.0


[-- Attachment #3: Type: text/plain, Size: 219 bytes --]


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple