From d389a33028d1a8a8520bc5c85dbcf767131fd51c Mon Sep 17 00:00:00 2001 From: Liang Zhu Date: Tue, 5 Dec 2017 03:49:11 -0800 Subject: [PATCH] exclude Unix Domain Socket from setErrMessageCB 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 | 16 ++++++ folly/io/async/test/AsyncSocketTest2.cpp | 69 ++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/folly/io/async/AsyncSocket.cpp b/folly/io/async/AsyncSocket.cpp index 1dbaa220..2d1e76cb 100644 --- a/folly/io/async/AsyncSocket.cpp +++ b/folly/io/async/AsyncSocket.cpp @@ -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; diff --git a/folly/io/async/test/AsyncSocketTest2.cpp b/folly/io/async/test/AsyncSocketTest2.cpp index 54034e30..ec67d2f2 100644 --- a/folly/io/async/test/AsyncSocketTest2.cpp +++ b/folly/io/async/test/AsyncSocketTest2.cpp @@ -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 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 -- 2.34.1