Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 7 Aug 2018 16:39:07 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r337424 - head/tests/sys/kern
Message-ID:  <201808071639.w77Gd74N026522@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: markj
Date: Tue Aug  7 16:39:07 2018
New Revision: 337424
URL: https://svnweb.freebsd.org/changeset/base/337424

Log:
  Update PR 131876 regression tests after r337423.
  
  - Add some more cases to the truncation test.
  - Remove the "expect fail" annotations.
  
  PR:		131876
  MFC after:	1 month
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D16562

Modified:
  head/tests/sys/kern/unix_passfd_test.c

Modified: head/tests/sys/kern/unix_passfd_test.c
==============================================================================
--- head/tests/sys/kern/unix_passfd_test.c	Tue Aug  7 16:36:48 2018	(r337423)
+++ head/tests/sys/kern/unix_passfd_test.c	Tue Aug  7 16:39:07 2018	(r337424)
@@ -119,6 +119,20 @@ getnfds(void)
 }
 
 static void
+putfds(char *buf, int fd, int nfds)
+{
+	struct cmsghdr *cm;
+	int *fdp, i;
+
+	cm = (struct cmsghdr *)buf;
+	cm->cmsg_len = CMSG_LEN(nfds * sizeof(int));
+	cm->cmsg_level = SOL_SOCKET;
+	cm->cmsg_type = SCM_RIGHTS;
+	for (fdp = (int *)CMSG_DATA(cm), i = 0; i < nfds; i++)
+		*fdp++ = fd;
+}
+
+static void
 samefile(struct stat *sb1, struct stat *sb2)
 {
 
@@ -131,7 +145,6 @@ sendfd_payload(int sockfd, int send_fd, void *payload,
 {
 	struct iovec iovec;
 	char message[CMSG_SPACE(sizeof(int))];
-	struct cmsghdr *cmsghdr;
 	struct msghdr msghdr;
 	ssize_t len;
 
@@ -147,13 +160,8 @@ sendfd_payload(int sockfd, int send_fd, void *payload,
 	msghdr.msg_iov = &iovec;
 	msghdr.msg_iovlen = 1;
 
-	cmsghdr = (struct cmsghdr *)(void *)message;
-	cmsghdr->cmsg_len = CMSG_LEN(sizeof(int));
-	cmsghdr->cmsg_level = SOL_SOCKET;
-	cmsghdr->cmsg_type = SCM_RIGHTS;
-	memcpy(CMSG_DATA(cmsghdr), &send_fd, sizeof(int));
-
-	len = sendmsg(sockfd, &msghdr, MSG_DONTWAIT);
+	putfds(message, send_fd, 1);
+	len = sendmsg(sockfd, &msghdr, 0);
 	ATF_REQUIRE_MSG(len != -1, "sendmsg failed: %s", strerror(errno));
 	return ((size_t)len);
 }
@@ -185,20 +193,22 @@ localcreds(int sockfd)
 }
 
 static void
-recvfd_payload(int sockfd, int *recv_fd, void *buf, size_t buflen)
+recvfd_payload(int sockfd, int *recv_fd, void *buf, size_t buflen,
+    size_t cmsgsz)
 {
 	struct cmsghdr *cmsghdr;
-	char message[CMSG_SPACE(SOCKCREDSIZE(CMGROUP_MAX)) +
-	    CMSG_SPACE(sizeof(int))];
 	struct msghdr msghdr;
 	struct iovec iovec;
+	char *message;
 	ssize_t len;
 	bool foundcreds;
 
 	bzero(&msghdr, sizeof(msghdr));
+	message = malloc(cmsgsz);
+	ATF_REQUIRE(message != NULL);
 
 	msghdr.msg_control = message;
-	msghdr.msg_controllen = sizeof(message);
+	msghdr.msg_controllen = cmsgsz;
 
 	iovec.iov_base = buf;
 	iovec.iov_len = buflen;
@@ -237,7 +247,8 @@ recvfd(int sockfd, int *recv_fd)
 {
 	char ch = 0;
 
-	recvfd_payload(sockfd, recv_fd, &ch, sizeof(ch));
+	recvfd_payload(sockfd, recv_fd, &ch, sizeof(ch),
+	    CMSG_SPACE(sizeof(int)));
 }
 
 /*
@@ -418,65 +429,159 @@ ATF_TC_BODY(rights_creds_payload, tc)
 
 	len = sendfd_payload(fd[0], putfd, buf, sendspace);
 	ATF_REQUIRE_MSG(len < sendspace, "sendmsg: %zu bytes sent", len);
-	recvfd_payload(fd[1], &getfd, buf, len);
+	recvfd_payload(fd[1], &getfd, buf, len,
+	    CMSG_SPACE(SOCKCREDSIZE(CMGROUP_MAX)) + CMSG_SPACE(sizeof(int)));
 
 	close(putfd);
 	close(getfd);
 	closesocketpair(fd);
 }
 
+static void
+send_cmsg(int sockfd, void *cmsg, size_t cmsgsz)
+{
+	struct iovec iov;
+	struct msghdr msghdr;
+	ssize_t len;
+	char ch;
+
+	ch = 0;
+	bzero(&msghdr, sizeof(msghdr));
+
+	iov.iov_base = &ch;
+	iov.iov_len = sizeof(ch);
+	msghdr.msg_control = cmsg;
+	msghdr.msg_controllen = cmsgsz;
+	msghdr.msg_iov = &iov;
+	msghdr.msg_iovlen = 1;
+
+	len = sendmsg(sockfd, &msghdr, 0);
+	ATF_REQUIRE_MSG(len != -1,
+	    "sendmsg failed: %s", strerror(errno));
+	ATF_REQUIRE_MSG(len == sizeof(ch),
+	    "sendmsg: %zd bytes sent; expected %zu", len, sizeof(ch));
+}
+
+static void
+recv_cmsg(int sockfd, char *cmsg, size_t cmsgsz, int flags)
+{
+	struct iovec iov;
+	struct msghdr msghdr;
+	ssize_t len;
+	char ch;
+
+	ch = 0;
+	bzero(&msghdr, sizeof(msghdr));
+
+	iov.iov_base = &ch;
+	iov.iov_len = sizeof(ch);
+	msghdr.msg_control = cmsg;
+	msghdr.msg_controllen = cmsgsz;
+	msghdr.msg_iov = &iov;
+	msghdr.msg_iovlen = 1;
+
+	len = recvmsg(sockfd, &msghdr, 0);
+	ATF_REQUIRE_MSG(len != -1,
+	    "recvmsg failed: %s", strerror(errno));
+	ATF_REQUIRE_MSG(len == sizeof(ch),
+	    "recvmsg: %zd bytes received; expected %zu", len, sizeof(ch));
+	ATF_REQUIRE_MSG((msghdr.msg_flags & flags) == flags,
+	    "recvmsg: got flags %#x; expected %#x", msghdr.msg_flags, flags);
+}
+
 /*
- * Test for PR 131876. Receiver uses a control message buffer that is too
+ * Test for PR 131876.  Receiver uses a control message buffer that is too
  * small for the incoming SCM_RIGHTS message, so the message is truncated.
  * The kernel must not leak the copied right into the receiver's namespace.
  */
 ATF_TC_WITHOUT_HEAD(truncated_rights);
 ATF_TC_BODY(truncated_rights, tc)
 {
-	struct iovec iovec;
-	struct msghdr msghdr;
-	char buf[16], message[CMSG_SPACE(0)];
-	ssize_t len;
-	int fd[2], nfds, putfd;
+	char *message;
+	int fd[2], nfds, putfd, rc;
 
-	atf_tc_expect_fail("PR 131876: "
-	    "FD leak when 'control' message is truncated");
-
-	memset(buf, 42, sizeof(buf));
 	domainsocketpair(fd);
 	devnull(&putfd);
 	nfds = getnfds();
 
-	len = sendfd_payload(fd[0], putfd, buf, sizeof(buf));
-	ATF_REQUIRE_MSG(len == sizeof(buf),
-	    "sendmsg: %zd bytes sent; expected %zu; %s", len, sizeof(buf),
-	    strerror(errno));
+	/*
+	 * Case 1: Send a single descriptor and truncate the message.
+	 */
+	message = malloc(CMSG_SPACE(sizeof(int)));
+	ATF_REQUIRE(message != NULL);
+	putfds(message, putfd, 1);
+	send_cmsg(fd[0], message, CMSG_LEN(sizeof(int)));
+	recv_cmsg(fd[1], message, CMSG_LEN(0), MSG_CTRUNC);
+	ATF_REQUIRE(getnfds() == nfds);
+	free(message);
 
-	bzero(&msghdr, sizeof(msghdr));
-	bzero(message, sizeof(message));
+	/*
+	 * Case 2a: Send two descriptors in separate messages, and truncate at
+	 *          the boundary between the two messages.  We should still
+	 *          receive the first message.
+	 */
+	message = malloc(CMSG_SPACE(sizeof(int)) * 2);
+	ATF_REQUIRE(message != NULL);
+	putfds(message, putfd, 1);
+	putfds(message + CMSG_SPACE(sizeof(int)), putfd, 1);
+	send_cmsg(fd[0], message, CMSG_SPACE(sizeof(int)) * 2);
+	recv_cmsg(fd[1], message, CMSG_SPACE(sizeof(int)), MSG_CTRUNC);
+	rc = close(*(int *)CMSG_DATA(message));
+	ATF_REQUIRE_MSG(rc == 0, "close failed: %s", strerror(errno));
+	ATF_REQUIRE(getnfds() == nfds);
+	free(message);
 
-	iovec.iov_base = buf;
-	iovec.iov_len = sizeof(buf);
-	msghdr.msg_control = message;
-	msghdr.msg_controllen = sizeof(message);
-	msghdr.msg_iov = &iovec;
-	msghdr.msg_iovlen = 1;
+	/*
+	 * Case 2b: Send two descriptors in separate messages, and truncate
+	 *          before the end of the first message.
+	 */
+	message = malloc(CMSG_SPACE(sizeof(int)) * 2);
+	ATF_REQUIRE(message != NULL);
+	putfds(message, putfd, 1);
+	putfds(message + CMSG_SPACE(sizeof(int)), putfd, 1);
+	send_cmsg(fd[0], message, CMSG_SPACE(sizeof(int)) * 2);
+	recv_cmsg(fd[1], message, CMSG_SPACE(0), MSG_CTRUNC);
+	ATF_REQUIRE(getnfds() == nfds);
+	free(message);
 
-	len = recvmsg(fd[1], &msghdr, 0);
-	ATF_REQUIRE_MSG(len != -1, "recvmsg failed: %s", strerror(errno));
-	ATF_REQUIRE_MSG((size_t)len == sizeof(buf),
-	    "recvmsg: %zd bytes received; expected %zd", len, sizeof(buf));
-	for (size_t i = 0; i < sizeof(buf); i++)
-		ATF_REQUIRE_MSG(buf[i] == 42, "unexpected buffer contents");
+	/*
+	 * Case 2c: Send two descriptors in separate messages, and truncate
+	 *          after the end of the first message.  We should still
+	 *          receive the first message.
+	 */
+	message = malloc(CMSG_SPACE(sizeof(int)) * 2);
+	ATF_REQUIRE(message != NULL);
+	putfds(message, putfd, 1);
+	putfds(message + CMSG_SPACE(sizeof(int)), putfd, 1);
+	send_cmsg(fd[0], message, CMSG_SPACE(sizeof(int)) * 2);
+	recv_cmsg(fd[1], message, CMSG_SPACE(sizeof(int)) + CMSG_SPACE(0),
+	    MSG_CTRUNC);
+	rc = close(*(int *)CMSG_DATA(message));
+	ATF_REQUIRE_MSG(rc == 0, "close failed: %s", strerror(errno));
+	ATF_REQUIRE(getnfds() == nfds);
+	free(message);
 
-	ATF_REQUIRE_MSG((msghdr.msg_flags & MSG_CTRUNC) != 0,
-	    "MSG_CTRUNC not set after truncation");
+	/*
+	 * Case 3: Send three descriptors in the same message, and leave space
+	 *         only for the first when receiving the message.
+	 */
+	message = malloc(CMSG_SPACE(sizeof(int) * 3));
+	ATF_REQUIRE(message != NULL);
+	putfds(message, putfd, 3);
+	send_cmsg(fd[0], message, CMSG_SPACE(sizeof(int) * 3));
+	recv_cmsg(fd[1], message, CMSG_SPACE(sizeof(int)), MSG_CTRUNC);
 	ATF_REQUIRE(getnfds() == nfds);
+	free(message);
 
 	close(putfd);
 	closesocketpair(fd);
 }
 
+/*
+ * Ensure that an attempt to copy a SCM_RIGHTS message to the recipient
+ * fails.  In this case the kernel must dispose of the externalized rights
+ * rather than leaking them into the recipient's file descriptor table.
+ */
 ATF_TC_WITHOUT_HEAD(copyout_rights_error);
 ATF_TC_BODY(copyout_rights_error, tc)
 {
@@ -485,9 +590,6 @@ ATF_TC_BODY(copyout_rights_error, tc)
 	char buf[16];
 	ssize_t len;
 	int fd[2], error, nfds, putfd;
-
-	atf_tc_expect_fail("PR 131876: "
-	    "FD leak when copyout of rights returns an error");
 
 	memset(buf, 0, sizeof(buf));
 	domainsocketpair(fd);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808071639.w77Gd74N026522>