exclude Unix Domain Socket from setErrMessageCB
authorLiang Zhu <liangzhu@fb.com>
Tue, 5 Dec 2017 11:49:11 +0000 (03:49 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Tue, 5 Dec 2017 11:50:15 +0000 (03:50 -0800)
Summary:
In the latest stable kernel 4.14.3 as of 2017-12-04, unix domain socket does not support MSG_ERRQUEUE. So recvmsg(MSG_ERRQUEUE) will read application data from unix doamin socket as error message, which breaks the message flow in application. This diff disable setErrMessageCB for Unix Domain Socket.

Both [[ https://github.com/torvalds/linux/blob/master/net/ipv4/tcp.c#L1782 | tcp_recvmsg ]] and [[ https://github.com/torvalds/linux/blob/master/net/ipv4/udp.c#L1571 | udp_recvmsg ]] will check flag MSG_ERRQUEUE and read from error queue.  However, there is nothing about MSG_ERRQUEUE in [[ https://github.com/torvalds/linux/blob/master/net/unix/af_unix.c#L2249 | af_unix ]].

Reviewed By: yfeldblum

Differential Revision: D6479465

fbshipit-source-id: eba2c8650e96466f2b361a42ddf90053d65f19bd

folly/io/async/AsyncSocket.cpp
folly/io/async/test/AsyncSocketTest2.cpp

index 1dbaa220ceca5987833c8b114235eec30cce21a1..2d1e76cbc86a40547c1d41a16375b73ad4c59b48 100644 (file)
@@ -662,6 +662,22 @@ void AsyncSocket::setErrMessageCB(ErrMessageCallback* callback) {
           << ", fd=" << fd_ << ", callback=" << callback
           << ", state=" << state_;
 
+  // In the latest stable kernel 4.14.3 as of 2017-12-04, unix domain
+  // socket does not support MSG_ERRQUEUE. So recvmsg(MSG_ERRQUEUE)
+  // will read application data from unix doamin socket as error
+  // message, which breaks the message flow in application.  Feel free
+  // to remove the next code block if MSG_ERRQUEUE is added for unix
+  // domain socket in the future.
+  if (callback != nullptr) {
+    cacheLocalAddress();
+    if (localAddr_.getFamily() == AF_UNIX) {
+      LOG(ERROR) << "Failed to set ErrMessageCallback=" << callback
+                 << " for Unix Doamin Socket where MSG_ERRQUEUE is unsupported,"
+                 << " fd=" << fd_;
+      return;
+    }
+  }
+
   // Short circuit if callback is the same as the existing errMessageCallback_.
   if (callback == errMessageCallback_) {
     return;
index 54034e305c0b70da1be17c458cbf667f0dafe759..ec67d2f21c17cb0e0f55e4801b57f42c5e7be90a 100644 (file)
@@ -3298,4 +3298,73 @@ TEST(AsyncSocketTest, SendMessageAncillaryData) {
       magicString.end(),
       transferredMagicString.begin()));
 }
+
+TEST(AsyncSocketTest, UnixDomainSocketErrMessageCB) {
+  // In the latest stable kernel 4.14.3 as of 2017-12-04, Unix Domain
+  // Socket (UDS) does not support MSG_ERRQUEUE. So
+  // recvmsg(MSG_ERRQUEUE) will read application data from UDS which
+  // breaks application message flow.  To avoid this problem,
+  // AsyncSocket currently disables setErrMessageCB for UDS.
+  //
+  // This tests two things for UDS
+  // 1. setErrMessageCB fails
+  // 2. recvmsg(MSG_ERRQUEUE) reads application data
+  //
+  // Feel free to remove this test if UDS supports MSG_ERRQUEUE in the future.
+
+  int fd[2];
+  EXPECT_EQ(socketpair(AF_UNIX, SOCK_STREAM, 0, fd), 0);
+  ASSERT_NE(fd[0], -1);
+  ASSERT_NE(fd[1], -1);
+  SCOPE_EXIT {
+    close(fd[1]);
+  };
+
+  EXPECT_EQ(fcntl(fd[0], F_SETFL, O_NONBLOCK), 0);
+  EXPECT_EQ(fcntl(fd[1], F_SETFL, O_NONBLOCK), 0);
+
+  EventBase evb;
+  std::shared_ptr<AsyncSocket> socket = AsyncSocket::newSocket(&evb, fd[0]);
+
+  // setErrMessageCB should fail for unix domain socket
+  TestErrMessageCallback errMsgCB;
+  ASSERT_NE(&errMsgCB, nullptr);
+  socket->setErrMessageCB(&errMsgCB);
+  ASSERT_EQ(socket->getErrMessageCallback(), nullptr);
+
+#ifdef FOLLY_HAVE_MSG_ERRQUEUE
+  // The following verifies that MSG_ERRQUEUE does not work for UDS,
+  // and recvmsg reads application data
+  union {
+    // Space large enough to hold an 'int'
+    char control[CMSG_SPACE(sizeof(int))];
+    struct cmsghdr cmh;
+  } r_u;
+  struct msghdr msgh;
+  struct iovec iov;
+  int recv_data = 0;
+
+  msgh.msg_control = r_u.control;
+  msgh.msg_controllen = sizeof(r_u.control);
+  msgh.msg_name = nullptr;
+  msgh.msg_namelen = 0;
+  msgh.msg_iov = &iov;
+  msgh.msg_iovlen = 1;
+  iov.iov_base = &recv_data;
+  iov.iov_len = sizeof(recv_data);
+
+  // there is no data, recvmsg should fail
+  EXPECT_EQ(recvmsg(fd[1], &msgh, MSG_ERRQUEUE), -1);
+  EXPECT_TRUE(errno == EAGAIN || errno == EWOULDBLOCK);
+
+  // provide some application data, error queue should be empty if it exists
+  // However, UDS reads application data as error message
+  int test_data = 123456;
+  WriteCallback wcb;
+  socket->write(&wcb, &test_data, sizeof(test_data));
+  recv_data = 0;
+  ASSERT_NE(recvmsg(fd[1], &msgh, MSG_ERRQUEUE), -1);
+  ASSERT_EQ(recv_data, test_data);
+#endif // FOLLY_HAVE_MSG_ERRQUEUE
+}
 #endif