[PATCH v1 net 00/11] af_unix: Fix bunch of MSG_OOB bugs and add new tests.

[PATCH v1 net 00/11] af_unix: Fix bunch of MSG_OOB bugs and add new tests.

[Kuniyuki Iwashima]

This series rewrites the selftest for AF_UNIX MSG_OOB and fixes
bunch of bugs that AF_UNIX behaves differently compared to TCP.

Note that the test discovered few more bugs in TCP side, which
will be fixed in another series.


Kuniyuki Iwashima (11):
  selftest: af_unix: Remove test_unix_oob.c.
  selftest: af_unix: Add msg_oob.c.
  af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.
  af_unix: Don't stop recv(MSG_DONTWAIT) if consumed OOB skb is at the
    head.
  selftest: af_unix: Add non-TCP-compliant test cases in msg_oob.c.
  af_unix: Don't stop recv() at consumed ex-OOB skb.
  selftest: af_unix: Add SO_OOBINLINE test cases in msg_oob.c
  selftest: af_unix: Check SIGURG after every send() in msg_oob.c
  selftest: af_unix: Check EPOLLPRI after every send()/recv() in
    msg_oob.c
  af_unix: Fix wrong ioctl(SIOCATMARK) when consumed OOB skb is at the
    head.
  selftest: af_unix: Check SIOCATMARK after every send()/recv() in
    msg_oob.c.

 net/unix/af_unix.c                            |  37 +-
 tools/testing/selftests/net/.gitignore        |   1 -
 tools/testing/selftests/net/af_unix/Makefile  |   2 +-
 tools/testing/selftests/net/af_unix/msg_oob.c | 734 ++++++++++++++++++
 .../selftests/net/af_unix/test_unix_oob.c     | 436 -----------
 5 files changed, 766 insertions(+), 444 deletions(-)
 create mode 100644 tools/testing/selftests/net/af_unix/msg_oob.c
 delete mode 100644 tools/testing/selftests/net/af_unix/test_unix_oob.c

-- 
2.30.2

[PATCH v1 net 01/11] selftest: af_unix: Remove test_unix_oob.c.

[Kuniyuki Iwashima]

test_unix_oob.c does not fully cover AF_UNIX's MSG_OOB functionality,
thus there are discrepancies between TCP behaviour.

Also, the test uses fork() to create message producer, and it's not
easy to understand and add more test cases.

Let's remove test_unix_oob.c and rewrite a new test.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 tools/testing/selftests/net/.gitignore        |   1 -
 tools/testing/selftests/net/af_unix/Makefile  |   2 +-
 .../selftests/net/af_unix/test_unix_oob.c     | 436 ------------------
 3 files changed, 1 insertion(+), 438 deletions(-)
 delete mode 100644 tools/testing/selftests/net/af_unix/test_unix_oob.c

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 49a56eb5d036..666ab7d9390b 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -43,7 +43,6 @@ tap
 tcp_fastopen_backup_key
 tcp_inq
 tcp_mmap
-test_unix_oob
 timestamping
 tls
 toeplitz
diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
index 3b83c797650d..a25845251eed 100644
--- a/tools/testing/selftests/net/af_unix/Makefile
+++ b/tools/testing/selftests/net/af_unix/Makefile
@@ -1,4 +1,4 @@
 CFLAGS += $(KHDR_INCLUDES)
-TEST_GEN_PROGS := diag_uid test_unix_oob unix_connect scm_pidfd scm_rights
+TEST_GEN_PROGS := diag_uid scm_pidfd scm_rights unix_connect
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/af_unix/test_unix_oob.c b/tools/testing/selftests/net/af_unix/test_unix_oob.c
deleted file mode 100644
index a7c51889acd5..000000000000
--- a/tools/testing/selftests/net/af_unix/test_unix_oob.c
+++ /dev/null
@@ -1,436 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/socket.h>
-#include <arpa/inet.h>
-#include <unistd.h>
-#include <string.h>
-#include <fcntl.h>
-#include <sys/ioctl.h>
-#include <errno.h>
-#include <netinet/tcp.h>
-#include <sys/un.h>
-#include <sys/signal.h>
-#include <sys/poll.h>
-
-static int pipefd[2];
-static int signal_recvd;
-static pid_t producer_id;
-static char sock_name[32];
-
-static void sig_hand(int sn, siginfo_t *si, void *p)
-{
-	signal_recvd = sn;
-}
-
-static int set_sig_handler(int signal)
-{
-	struct sigaction sa;
-
-	sa.sa_sigaction = sig_hand;
-	sigemptyset(&sa.sa_mask);
-	sa.sa_flags = SA_SIGINFO | SA_RESTART;
-
-	return sigaction(signal, &sa, NULL);
-}
-
-static void set_filemode(int fd, int set)
-{
-	int flags = fcntl(fd, F_GETFL, 0);
-
-	if (set)
-		flags &= ~O_NONBLOCK;
-	else
-		flags |= O_NONBLOCK;
-	fcntl(fd, F_SETFL, flags);
-}
-
-static void signal_producer(int fd)
-{
-	char cmd;
-
-	cmd = 'S';
-	write(fd, &cmd, sizeof(cmd));
-}
-
-static void wait_for_signal(int fd)
-{
-	char buf[5];
-
-	read(fd, buf, 5);
-}
-
-static void die(int status)
-{
-	fflush(NULL);
-	unlink(sock_name);
-	kill(producer_id, SIGTERM);
-	exit(status);
-}
-
-int is_sioctatmark(int fd)
-{
-	int ans = -1;
-
-	if (ioctl(fd, SIOCATMARK, &ans, sizeof(ans)) < 0) {
-#ifdef DEBUG
-		perror("SIOCATMARK Failed");
-#endif
-	}
-	return ans;
-}
-
-void read_oob(int fd, char *c)
-{
-
-	*c = ' ';
-	if (recv(fd, c, sizeof(*c), MSG_OOB) < 0) {
-#ifdef DEBUG
-		perror("Reading MSG_OOB Failed");
-#endif
-	}
-}
-
-int read_data(int pfd, char *buf, int size)
-{
-	int len = 0;
-
-	memset(buf, size, '0');
-	len = read(pfd, buf, size);
-#ifdef DEBUG
-	if (len < 0)
-		perror("read failed");
-#endif
-	return len;
-}
-
-static void wait_for_data(int pfd, int event)
-{
-	struct pollfd pfds[1];
-
-	pfds[0].fd = pfd;
-	pfds[0].events = event;
-	poll(pfds, 1, -1);
-}
-
-void producer(struct sockaddr_un *consumer_addr)
-{
-	int cfd;
-	char buf[64];
-	int i;
-
-	memset(buf, 'x', sizeof(buf));
-	cfd = socket(AF_UNIX, SOCK_STREAM, 0);
-
-	wait_for_signal(pipefd[0]);
-	if (connect(cfd, (struct sockaddr *)consumer_addr,
-		     sizeof(*consumer_addr)) != 0) {
-		perror("Connect failed");
-		kill(0, SIGTERM);
-		exit(1);
-	}
-
-	for (i = 0; i < 2; i++) {
-		/* Test 1: Test for SIGURG and OOB */
-		wait_for_signal(pipefd[0]);
-		memset(buf, 'x', sizeof(buf));
-		buf[63] = '@';
-		send(cfd, buf, sizeof(buf), MSG_OOB);
-
-		wait_for_signal(pipefd[0]);
-
-		/* Test 2: Test for OOB being overwitten */
-		memset(buf, 'x', sizeof(buf));
-		buf[63] = '%';
-		send(cfd, buf, sizeof(buf), MSG_OOB);
-
-		memset(buf, 'x', sizeof(buf));
-		buf[63] = '#';
-		send(cfd, buf, sizeof(buf), MSG_OOB);
-
-		wait_for_signal(pipefd[0]);
-
-		/* Test 3: Test for SIOCATMARK */
-		memset(buf, 'x', sizeof(buf));
-		buf[63] = '@';
-		send(cfd, buf, sizeof(buf), MSG_OOB);
-
-		memset(buf, 'x', sizeof(buf));
-		buf[63] = '%';
-		send(cfd, buf, sizeof(buf), MSG_OOB);
-
-		memset(buf, 'x', sizeof(buf));
-		send(cfd, buf, sizeof(buf), 0);
-
-		wait_for_signal(pipefd[0]);
-
-		/* Test 4: Test for 1byte OOB msg */
-		memset(buf, 'x', sizeof(buf));
-		buf[0] = '@';
-		send(cfd, buf, 1, MSG_OOB);
-	}
-}
-
-int
-main(int argc, char **argv)
-{
-	int lfd, pfd;
-	struct sockaddr_un consumer_addr, paddr;
-	socklen_t len = sizeof(consumer_addr);
-	char buf[1024];
-	int on = 0;
-	char oob;
-	int atmark;
-
-	lfd = socket(AF_UNIX, SOCK_STREAM, 0);
-	memset(&consumer_addr, 0, sizeof(consumer_addr));
-	consumer_addr.sun_family = AF_UNIX;
-	sprintf(sock_name, "unix_oob_%d", getpid());
-	unlink(sock_name);
-	strcpy(consumer_addr.sun_path, sock_name);
-
-	if ((bind(lfd, (struct sockaddr *)&consumer_addr,
-		  sizeof(consumer_addr))) != 0) {
-		perror("socket bind failed");
-		exit(1);
-	}
-
-	pipe(pipefd);
-
-	listen(lfd, 1);
-
-	producer_id = fork();
-	if (producer_id == 0) {
-		producer(&consumer_addr);
-		exit(0);
-	}
-
-	set_sig_handler(SIGURG);
-	signal_producer(pipefd[1]);
-
-	pfd = accept(lfd, (struct sockaddr *) &paddr, &len);
-	fcntl(pfd, F_SETOWN, getpid());
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 1:
-	 * veriyf that SIGURG is
-	 * delivered, 63 bytes are
-	 * read, oob is '@', and POLLPRI works.
-	 */
-	wait_for_data(pfd, POLLPRI);
-	read_oob(pfd, &oob);
-	len = read_data(pfd, buf, 1024);
-	if (!signal_recvd || len != 63 || oob != '@') {
-		fprintf(stderr, "Test 1 failed sigurg %d len %d %c\n",
-			 signal_recvd, len, oob);
-			die(1);
-	}
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 2:
-	 * Verify that the first OOB is over written by
-	 * the 2nd one and the first OOB is returned as
-	 * part of the read, and sigurg is received.
-	 */
-	wait_for_data(pfd, POLLIN | POLLPRI);
-	len = 0;
-	while (len < 70)
-		len = recv(pfd, buf, 1024, MSG_PEEK);
-	len = read_data(pfd, buf, 1024);
-	read_oob(pfd, &oob);
-	if (!signal_recvd || len != 127 || oob != '#') {
-		fprintf(stderr, "Test 2 failed, sigurg %d len %d OOB %c\n",
-		signal_recvd, len, oob);
-		die(1);
-	}
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 3:
-	 * verify that 2nd oob over writes
-	 * the first one and read breaks at
-	 * oob boundary returning 127 bytes
-	 * and sigurg is received and atmark
-	 * is set.
-	 * oob is '%' and second read returns
-	 * 64 bytes.
-	 */
-	len = 0;
-	wait_for_data(pfd, POLLIN | POLLPRI);
-	while (len < 150)
-		len = recv(pfd, buf, 1024, MSG_PEEK);
-	len = read_data(pfd, buf, 1024);
-	atmark = is_sioctatmark(pfd);
-	read_oob(pfd, &oob);
-
-	if (!signal_recvd || len != 127 || oob != '%' || atmark != 1) {
-		fprintf(stderr,
-			"Test 3 failed, sigurg %d len %d OOB %c atmark %d\n",
-			signal_recvd, len, oob, atmark);
-		die(1);
-	}
-
-	signal_recvd = 0;
-
-	len = read_data(pfd, buf, 1024);
-	if (len != 64) {
-		fprintf(stderr, "Test 3.1 failed, sigurg %d len %d OOB %c\n",
-			signal_recvd, len, oob);
-		die(1);
-	}
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 4:
-	 * verify that a single byte
-	 * oob message is delivered.
-	 * set non blocking mode and
-	 * check proper error is
-	 * returned and sigurg is
-	 * received and correct
-	 * oob is read.
-	 */
-
-	set_filemode(pfd, 0);
-
-	wait_for_data(pfd, POLLIN | POLLPRI);
-	len = read_data(pfd, buf, 1024);
-	if ((len == -1) && (errno == 11))
-		len = 0;
-
-	read_oob(pfd, &oob);
-
-	if (!signal_recvd || len != 0 || oob != '@') {
-		fprintf(stderr, "Test 4 failed, sigurg %d len %d OOB %c\n",
-			 signal_recvd, len, oob);
-		die(1);
-	}
-
-	set_filemode(pfd, 1);
-
-	/* Inline Testing */
-
-	on = 1;
-	if (setsockopt(pfd, SOL_SOCKET, SO_OOBINLINE, &on, sizeof(on))) {
-		perror("SO_OOBINLINE");
-		die(1);
-	}
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 1 -- Inline:
-	 * Check that SIGURG is
-	 * delivered and 63 bytes are
-	 * read and oob is '@'
-	 */
-
-	wait_for_data(pfd, POLLIN | POLLPRI);
-	len = read_data(pfd, buf, 1024);
-
-	if (!signal_recvd || len != 63) {
-		fprintf(stderr, "Test 1 Inline failed, sigurg %d len %d\n",
-			signal_recvd, len);
-		die(1);
-	}
-
-	len = read_data(pfd, buf, 1024);
-
-	if (len != 1) {
-		fprintf(stderr,
-			 "Test 1.1 Inline failed, sigurg %d len %d oob %c\n",
-			 signal_recvd, len, oob);
-		die(1);
-	}
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 2 -- Inline:
-	 * Verify that the first OOB is over written by
-	 * the 2nd one and read breaks correctly on
-	 * 2nd OOB boundary with the first OOB returned as
-	 * part of the read, and sigurg is delivered and
-	 * siocatmark returns true.
-	 * next read returns one byte, the oob byte
-	 * and siocatmark returns false.
-	 */
-	len = 0;
-	wait_for_data(pfd, POLLIN | POLLPRI);
-	while (len < 70)
-		len = recv(pfd, buf, 1024, MSG_PEEK);
-	len = read_data(pfd, buf, 1024);
-	atmark = is_sioctatmark(pfd);
-	if (len != 127 || atmark != 1 || !signal_recvd) {
-		fprintf(stderr, "Test 2 Inline failed, len %d atmark %d\n",
-			 len, atmark);
-		die(1);
-	}
-
-	len = read_data(pfd, buf, 1024);
-	atmark = is_sioctatmark(pfd);
-	if (len != 1 || buf[0] != '#' || atmark == 1) {
-		fprintf(stderr, "Test 2.1 Inline failed, len %d data %c atmark %d\n",
-			len, buf[0], atmark);
-		die(1);
-	}
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 3 -- Inline:
-	 * verify that 2nd oob over writes
-	 * the first one and read breaks at
-	 * oob boundary returning 127 bytes
-	 * and sigurg is received and siocatmark
-	 * is true after the read.
-	 * subsequent read returns 65 bytes
-	 * because of oob which should be '%'.
-	 */
-	len = 0;
-	wait_for_data(pfd, POLLIN | POLLPRI);
-	while (len < 126)
-		len = recv(pfd, buf, 1024, MSG_PEEK);
-	len = read_data(pfd, buf, 1024);
-	atmark = is_sioctatmark(pfd);
-	if (!signal_recvd || len != 127 || !atmark) {
-		fprintf(stderr,
-			 "Test 3 Inline failed, sigurg %d len %d data %c\n",
-			 signal_recvd, len, buf[0]);
-		die(1);
-	}
-
-	len = read_data(pfd, buf, 1024);
-	atmark = is_sioctatmark(pfd);
-	if (len != 65 || buf[0] != '%' || atmark != 0) {
-		fprintf(stderr,
-			 "Test 3.1 Inline failed, len %d oob %c atmark %d\n",
-			 len, buf[0], atmark);
-		die(1);
-	}
-
-	signal_recvd = 0;
-	signal_producer(pipefd[1]);
-
-	/* Test 4 -- Inline:
-	 * verify that a single
-	 * byte oob message is delivered
-	 * and read returns one byte, the oob
-	 * byte and sigurg is received
-	 */
-	wait_for_data(pfd, POLLIN | POLLPRI);
-	len = read_data(pfd, buf, 1024);
-	if (!signal_recvd || len != 1 || buf[0] != '@') {
-		fprintf(stderr,
-			"Test 4 Inline failed, signal %d len %d data %c\n",
-		signal_recvd, len, buf[0]);
-		die(1);
-	}
-	die(0);
-}
-- 
2.30.2

[PATCH v1 net 02/11] selftest: af_unix: Add msg_oob.c.

[Kuniyuki Iwashima]

AF_UNIX's MSG_OOB functionality lacked thorough testing, and we found
some bizarre behaviour.

The new selftest validates every MSG_OOB operation against TCP as a
reference implementation.

This patch adds only a few tests with basic send() and recv() that
do not fail.

The following patches will add more test cases for SO_OOBINLINE, SIGURG,
EPOLLPRI, and SIOCATMARK.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 tools/testing/selftests/net/af_unix/Makefile  |   2 +-
 tools/testing/selftests/net/af_unix/msg_oob.c | 220 ++++++++++++++++++
 2 files changed, 221 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/af_unix/msg_oob.c

diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
index a25845251eed..50584479540b 100644
--- a/tools/testing/selftests/net/af_unix/Makefile
+++ b/tools/testing/selftests/net/af_unix/Makefile
@@ -1,4 +1,4 @@
 CFLAGS += $(KHDR_INCLUDES)
-TEST_GEN_PROGS := diag_uid scm_pidfd scm_rights unix_connect
+TEST_GEN_PROGS := diag_uid msg_oob scm_pidfd scm_rights unix_connect
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
new file mode 100644
index 000000000000..d427d39d0806
--- /dev/null
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -0,0 +1,220 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright Amazon.com Inc. or its affiliates. */
+
+#include <fcntl.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include "../../kselftest_harness.h"
+
+#define BUF_SZ	32
+
+FIXTURE(msg_oob)
+{
+	int fd[4];		/* 0: AF_UNIX sender
+				 * 1: AF_UNIX receiver
+				 * 2: TCP sender
+				 * 3: TCP receiver
+				 */
+};
+
+static void create_unix_socketpair(struct __test_metadata *_metadata,
+				   FIXTURE_DATA(msg_oob) *self)
+{
+	int ret;
+
+	ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0, self->fd);
+	ASSERT_EQ(ret, 0);
+}
+
+static void create_tcp_socketpair(struct __test_metadata *_metadata,
+				  FIXTURE_DATA(msg_oob) *self)
+{
+	struct sockaddr_in addr;
+	socklen_t addrlen;
+	int listen_fd;
+	int ret;
+
+	listen_fd = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(listen_fd, 0);
+
+	ret = listen(listen_fd, -1);
+	ASSERT_EQ(ret, 0);
+
+	addrlen = sizeof(addr);
+	ret = getsockname(listen_fd, (struct sockaddr *)&addr, &addrlen);
+	ASSERT_EQ(ret, 0);
+
+	self->fd[2] = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(self->fd[2], 0);
+
+	ret = connect(self->fd[2], (struct sockaddr *)&addr, addrlen);
+	ASSERT_EQ(ret, 0);
+
+	self->fd[3] = accept(listen_fd, (struct sockaddr *)&addr, &addrlen);
+	ASSERT_GE(self->fd[3], 0);
+
+	ret = fcntl(self->fd[3], F_SETFL, O_NONBLOCK);
+	ASSERT_EQ(ret, 0);
+}
+
+static void close_sockets(FIXTURE_DATA(msg_oob) *self)
+{
+	int i;
+
+	for (i = 0; i < 4; i++)
+		close(self->fd[i]);
+}
+
+FIXTURE_SETUP(msg_oob)
+{
+	create_unix_socketpair(_metadata, self);
+	create_tcp_socketpair(_metadata, self);
+}
+
+FIXTURE_TEARDOWN(msg_oob)
+{
+	close_sockets(self);
+}
+
+static void __sendpair(struct __test_metadata *_metadata,
+		       FIXTURE_DATA(msg_oob) *self,
+		       const void *buf, size_t len, int flags)
+{
+	int i, ret[2];
+
+	for (i = 0; i < 2; i++)
+		ret[i] = send(self->fd[i * 2], buf, len, flags);
+
+	ASSERT_EQ(ret[0], len);
+	ASSERT_EQ(ret[0], ret[1]);
+}
+
+static void __recvpair(struct __test_metadata *_metadata,
+		       FIXTURE_DATA(msg_oob) *self,
+		       const void *expected_buf, int expected_len,
+		       int buf_len, int flags)
+{
+	int i, ret[2], recv_errno[2], expected_errno = 0;
+	char recv_buf[2][BUF_SZ] = {};
+
+	ASSERT_GE(BUF_SZ, buf_len);
+
+	errno = 0;
+
+	for (i = 0; i < 2; i++) {
+		ret[i] = recv(self->fd[i * 2 + 1], recv_buf[i], buf_len, flags);
+		recv_errno[i] = errno;
+	}
+
+	if (expected_len < 0) {
+		expected_errno = -expected_len;
+		expected_len = -1;
+	}
+
+	if (ret[0] != expected_len || recv_errno[0] != expected_errno) {
+		TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
+		TH_LOG("Expected:%s", expected_errno ? strerror(expected_errno) : expected_buf);
+
+		ASSERT_EQ(ret[0], expected_len);
+		ASSERT_EQ(recv_errno[0], expected_errno);
+	}
+
+	if (ret[0] != ret[1] || recv_errno[0] != recv_errno[1]) {
+		TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
+		TH_LOG("TCP     :%s", ret[1] < 0 ? strerror(recv_errno[1]) : recv_buf[1]);
+
+		ASSERT_EQ(ret[0], ret[1]);
+		ASSERT_EQ(recv_errno[0], recv_errno[1]);
+	}
+
+	if (expected_len >= 0) {
+		int cmp;
+
+		cmp = strncmp(expected_buf, recv_buf[0], expected_len);
+		if (cmp) {
+			TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
+			TH_LOG("Expected:%s", expected_errno ? strerror(expected_errno) : expected_buf);
+
+			ASSERT_EQ(cmp, 0);
+		}
+
+		cmp = strncmp(recv_buf[0], recv_buf[1], expected_len);
+		if (cmp) {
+			TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
+			TH_LOG("TCP     :%s", ret[1] < 0 ? strerror(recv_errno[1]) : recv_buf[1]);
+
+			ASSERT_EQ(cmp, 0);
+		}
+	}
+}
+
+#define sendpair(buf, len, flags)					\
+	__sendpair(_metadata, self, buf, len, flags)
+
+#define recvpair(expected_buf, expected_len, buf_len, flags)		\
+	__recvpair(_metadata, self,					\
+		   expected_buf, expected_len, buf_len, flags)
+
+TEST_F(msg_oob, non_oob)
+{
+	sendpair("x", 1, 0);
+
+	recvpair("", -EINVAL, 1, MSG_OOB);
+}
+
+TEST_F(msg_oob, oob)
+{
+	sendpair("x", 1, MSG_OOB);
+
+	recvpair("x", 1, 1, MSG_OOB);
+}
+
+TEST_F(msg_oob, oob_drop)
+{
+	sendpair("x", 1, MSG_OOB);
+
+	recvpair("", -EAGAIN, 1, 0);		/* Drop OOB. */
+	recvpair("", -EINVAL, 1, MSG_OOB);
+}
+
+TEST_F(msg_oob, oob_ahead)
+{
+	sendpair("hello", 5, MSG_OOB);
+
+	recvpair("o", 1, 1, MSG_OOB);
+	recvpair("hell", 4, 4, 0);
+}
+
+TEST_F(msg_oob, oob_break)
+{
+	sendpair("hello", 5, MSG_OOB);
+
+	recvpair("hell", 4, 5, 0);		/* Break at OOB even with enough buffer. */
+	recvpair("o", 1, 1, MSG_OOB);
+}
+
+TEST_F(msg_oob, oob_ahead_break)
+{
+	sendpair("hello", 5, MSG_OOB);
+	sendpair("world", 5, 0);
+
+	recvpair("o", 1, 1, MSG_OOB);
+	recvpair("hell", 4, 9, 0);		/* Break at OOB even after it's recv()ed. */
+	recvpair("world", 5, 5, 0);
+}
+
+TEST_F(msg_oob, oob_break_drop)
+{
+	sendpair("hello", 5, MSG_OOB);
+	sendpair("world", 5, 0);
+
+	recvpair("hell", 4, 10, 0);		/* Break at OOB even with enough buffer. */
+	recvpair("world", 5, 10, 0);		/* Drop OOB and recv() the next skb. */
+	recvpair("", -EINVAL, 1, MSG_OOB);
+}
+
+TEST_HARNESS_MAIN
-- 
2.30.2

Re: [PATCH v1 net 02/11] selftest: af_unix: Add msg_oob.c.

[Jakub Kicinski]

On Mon, 24 Jun 2024 18:36:36 -0700 Kuniyuki Iwashima wrote:
> +	if (ret[0] != expected_len || recv_errno[0] != expected_errno) {
> +		TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
> +		TH_LOG("Expected:%s", expected_errno ? strerror(expected_errno) : expected_buf);
> +
> +		ASSERT_EQ(ret[0], expected_len);
> +		ASSERT_EQ(recv_errno[0], expected_errno);
> +	}

repeating the conditions feels slightly imperfect.
Would it be possible to modify EXPECT_* to return the condition?
Then we could:

	if (EXPECT(...)) {
		TH_LOG(...
		TH_LOG(...
	}

Re: [PATCH v1 net 02/11] selftest: af_unix: Add msg_oob.c.

[Kuniyuki Iwashima]

From: Jakub Kicinski <kuba@kernel.org>
Date: Tue, 25 Jun 2024 17:44:49 -0700
> On Mon, 24 Jun 2024 18:36:36 -0700 Kuniyuki Iwashima wrote:
> > +	if (ret[0] != expected_len || recv_errno[0] != expected_errno) {
> > +		TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
> > +		TH_LOG("Expected:%s", expected_errno ? strerror(expected_errno) : expected_buf);
> > +
> > +		ASSERT_EQ(ret[0], expected_len);
> > +		ASSERT_EQ(recv_errno[0], expected_errno);
> > +	}
> 
> repeating the conditions feels slightly imperfect.

Yeah actually I don't like this...

> Would it be possible to modify EXPECT_* to return the condition?
> Then we could:
> 
> 	if (EXPECT(...)) {
> 		TH_LOG(...
> 		TH_LOG(...
> 	}

We can use EXPECT_EQ() {} here, but for some test cases where TCP is
buggy, I'd like to print the difference but let the test pass.

For example, see patch 6.

  #  RUN           msg_oob.no_peek.ex_oob_ahead_break ...
  # msg_oob.c:146:ex_oob_ahead_break:AF_UNIX :hellowol
  # msg_oob.c:147:ex_oob_ahead_break:TCP     :helloworl
                                                     ^
            TCP recv()s already recv()ed data, "r" --'

  #            OK  msg_oob.no_peek.ex_oob_ahead_break
  ok 11 msg_oob.no_peek.ex_oob_ahead_break

In this case, this does not print the recv()ed data,

  if (self->tcp_compliant) {
      EXPECT_EQ(...) {
          /* log retval, errno, buffer */
      }
  }

and this fails the test even though AF_UNIX is doing correct.

  EXPECT_EQ(...) {
      if (self->tcp_compliant) {
          /* log retval, errno, buffer */
      }
  }

I think we can convert it to EXPECT_EQ() {} in all places after
fixing TCP side and removing tcp_incompliant{} uses in the test.

Re: [PATCH v1 net 02/11] selftest: af_unix: Add msg_oob.c.

[Jakub Kicinski]

On Tue, 25 Jun 2024 18:45:55 -0700 Kuniyuki Iwashima wrote:
> We can use EXPECT_EQ() {} here, but for some test cases where TCP is
> buggy, I'd like to print the difference but let the test pass.
...
> I think we can convert it to EXPECT_EQ() {} in all places after
> fixing TCP side and removing tcp_incompliant{} uses in the test.

I see, makes sense

Re: [PATCH v1 net 02/11] selftest: af_unix: Add msg_oob.c.

[kernel test robot]

Hi Kuniyuki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/selftest-af_unix-Remove-test_unix_oob-c/20240626-033725
base:   net/main
patch link:    https://lore.kernel.org/r/20240625013645.45034-3-kuniyu%40amazon.com
patch subject: [PATCH v1 net 02/11] selftest: af_unix: Add msg_oob.c.
:::::: branch date: 22 hours ago
:::::: commit date: 22 hours ago
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240627/202406270118.ajdL2qcN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202406270118.ajdL2qcN-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from msg_oob.c:11:
   msg_oob.c: In function '__recvpair':
>> ../../kselftest_harness.h:119:40: warning: format '%s' expects argument of type 'char *', but argument 6 has type 'const void *' [-Wformat=]
     119 |                 fprintf(TH_LOG_STREAM, "# %s:%d:%s:" fmt "\n", \
         |                                        ^~~~~~~~~~~~~
   ../../kselftest_harness.h:114:17: note: in expansion of macro '__TH_LOG'
     114 |                 __TH_LOG(fmt, ##__VA_ARGS__); \
         |                 ^~~~~~~~
   msg_oob.c:120:17: note: in expansion of macro 'TH_LOG'
     120 |                 TH_LOG("Expected:%s", expected_errno ? strerror(expected_errno) : expected_buf);
         |                 ^~~~~~
>> ../../kselftest_harness.h:119:40: warning: format '%s' expects argument of type 'char *', but argument 6 has type 'const void *' [-Wformat=]
     119 |                 fprintf(TH_LOG_STREAM, "# %s:%d:%s:" fmt "\n", \
         |                                        ^~~~~~~~~~~~~
   ../../kselftest_harness.h:114:17: note: in expansion of macro '__TH_LOG'
     114 |                 __TH_LOG(fmt, ##__VA_ARGS__); \
         |                 ^~~~~~~~
   msg_oob.c:140:25: note: in expansion of macro 'TH_LOG'
     140 |                         TH_LOG("Expected:%s", expected_errno ? strerror(expected_errno) : expected_buf);
         |                         ^~~~~~

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v1 net 03/11] af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.

[Kuniyuki Iwashima]

After consuming OOB data, recv() reading the preceding data must break at
the OOB skb regardless of MSG_PEEK.

Currently, MSG_PEEK does not stop recv() for AF_UNIX, and the behaviour is
not compliant with TCP.

  >>> from socket import *
  >>> c1, c2 = socketpair(AF_UNIX)
  >>> c1.send(b'hello', MSG_OOB)
  5
  >>> c1.send(b'world')
  5
  >>> c2.recv(1, MSG_OOB)
  b'o'
  >>> c2.recv(9, MSG_PEEK)  # This should return b'hell'
  b'hellworld'              # even with enough buffer.

Let's fix it by returning NULL for consumed skb and unlinking it only if
MSG_PEEK is not specified.

This patch also adds test cases that add recv(MSG_PEEK) before each recv().

Without fix:

  #  RUN           msg_oob.peek.oob_ahead_break ...
  # msg_oob.c:134:oob_ahead_break:AF_UNIX :hellworld
  # msg_oob.c:135:oob_ahead_break:Expected:hell
  # msg_oob.c:137:oob_ahead_break:Expected ret[0] (9) == expected_len (4)
  # oob_ahead_break: Test terminated by assertion
  #          FAIL  msg_oob.peek.oob_ahead_break
  not ok 13 msg_oob.peek.oob_ahead_break

With fix:

  #  RUN           msg_oob.peek.oob_ahead_break ...
  #            OK  msg_oob.peek.oob_ahead_break
  ok 13 msg_oob.peek.oob_ahead_break

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c                            |  9 ++++---
 tools/testing/selftests/net/af_unix/msg_oob.c | 25 +++++++++++++++++--
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 5e695a9a609c..2eaecf9d78a4 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2613,9 +2613,12 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 {
 	struct unix_sock *u = unix_sk(sk);
 
-	if (!unix_skb_len(skb) && !(flags & MSG_PEEK)) {
-		skb_unlink(skb, &sk->sk_receive_queue);
-		consume_skb(skb);
+	if (!unix_skb_len(skb)) {
+		if (!(flags & MSG_PEEK)) {
+			skb_unlink(skb, &sk->sk_receive_queue);
+			consume_skb(skb);
+		}
+
 		skb = NULL;
 	} else {
 		struct sk_buff *unlinked_skb = NULL;
diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index d427d39d0806..de8d1fcde883 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -21,6 +21,21 @@ FIXTURE(msg_oob)
 				 */
 };
 
+FIXTURE_VARIANT(msg_oob)
+{
+	bool peek;
+};
+
+FIXTURE_VARIANT_ADD(msg_oob, no_peek)
+{
+	.peek = false,
+};
+
+FIXTURE_VARIANT_ADD(msg_oob, peek)
+{
+	.peek = true
+};
+
 static void create_unix_socketpair(struct __test_metadata *_metadata,
 				   FIXTURE_DATA(msg_oob) *self)
 {
@@ -156,8 +171,14 @@ static void __recvpair(struct __test_metadata *_metadata,
 	__sendpair(_metadata, self, buf, len, flags)
 
 #define recvpair(expected_buf, expected_len, buf_len, flags)		\
-	__recvpair(_metadata, self,					\
-		   expected_buf, expected_len, buf_len, flags)
+	do {								\
+		if (variant->peek)					\
+			__recvpair(_metadata, self,			\
+				   expected_buf, expected_len,		\
+				   buf_len, (flags) | MSG_PEEK);	\
+		__recvpair(_metadata, self,				\
+			   expected_buf, expected_len, buf_len, flags);	\
+	} while (0)
 
 TEST_F(msg_oob, non_oob)
 {
-- 
2.30.2

Re: [PATCH v1 net 03/11] af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.

[Paolo Abeni]

On Mon, 2024-06-24 at 18:36 -0700, Kuniyuki Iwashima wrote:
> After consuming OOB data, recv() reading the preceding data must break at
> the OOB skb regardless of MSG_PEEK.
> 
> Currently, MSG_PEEK does not stop recv() for AF_UNIX, and the behaviour is
> not compliant with TCP.

I'm unsure we can change the MSG_PEEK behavior at this point: existing
application(s?) could relay on that, regardless of how inconsistent
such behavior is.

I think we need at least an explicit ack from Rao, as the main known
user.

Paolo

Re: [PATCH v1 net 03/11] af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.

[Paolo Abeni]

On Wed, 2024-06-26 at 18:56 +0200, Paolo Abeni wrote:
> On Mon, 2024-06-24 at 18:36 -0700, Kuniyuki Iwashima wrote:
> > After consuming OOB data, recv() reading the preceding data must break at
> > the OOB skb regardless of MSG_PEEK.
> > 
> > Currently, MSG_PEEK does not stop recv() for AF_UNIX, and the behaviour is
> > not compliant with TCP.
> 
> I'm unsure we can change the MSG_PEEK behavior at this point: existing
> application(s?) could relay on that, regardless of how inconsistent
> such behavior is.
> 
> I think we need at least an explicit ack from Rao, as the main known
> user.

I see Rao stated that the unix OoB implementation was designed to mimic
the tcp one:

https://lore.kernel.org/netdev/c5f6abbe-de43-48b8-856a-36ded227e94f@oracle.com/

so the series should be ok.

Still given the size of the changes and the behavior change I'm
wondering if the series should target net-next instead.
That would offer some time cushion to deal with eventual regression.
WDYT?

Thanks,

Paolo

Re: [PATCH v1 net 03/11] af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.

[Kuniyuki Iwashima]

From: Paolo Abeni <pabeni@redhat.com>
Date: Wed, 26 Jun 2024 23:10:40 +0200
> On Wed, 2024-06-26 at 18:56 +0200, Paolo Abeni wrote:
> > On Mon, 2024-06-24 at 18:36 -0700, Kuniyuki Iwashima wrote:
> > > After consuming OOB data, recv() reading the preceding data must break at
> > > the OOB skb regardless of MSG_PEEK.
> > > 
> > > Currently, MSG_PEEK does not stop recv() for AF_UNIX, and the behaviour is
> > > not compliant with TCP.
> > 
> > I'm unsure we can change the MSG_PEEK behavior at this point: existing
> > application(s?) could relay on that, regardless of how inconsistent
> > such behavior is.
> > 
> > I think we need at least an explicit ack from Rao, as the main known
> > user.
> 
> I see Rao stated that the unix OoB implementation was designed to mimic
> the tcp one:
> 
> https://lore.kernel.org/netdev/c5f6abbe-de43-48b8-856a-36ded227e94f@oracle.com/
> 
> so the series should be ok.
> 
> Still given the size of the changes and the behavior change I'm
> wondering if the series should target net-next instead.
> That would offer some time cushion to deal with eventual regression.
> WDYT?

The actual change is 37 LoC and we recently have this kind of changes
(30 LoC in total) in net.git.  The last two were merged in April and
we have no user report so far.

  a6736a0addd6 af_unix: Read with MSG_PEEK loops if the first unread byte is OOB
  22dd70eb2c3d af_unix: Don't peek OOB data without MSG_OOB.
  283454c8a123 af_unix: Call manage_oob() for every skb in unix_stream_read_generic().

Most of the changes are due to selftest.  The original test repeated the
same set of code but covered few cases.  OTOH, the new test spends most
of lines to add as many test cases as possible, which IMHO nicely covers
regression if we want to mimic TCP.

  On net.git:

  # FAILED: 20 / 38 tests passed.
  # Totals: pass:20 fail:18 xfail:0 xpass:0 skip:0 error:0

Also, now the original test is broken in stable after the commits above,
so I think it would be nice to have this series in stable.

Thanks!

Re: [PATCH v1 net 03/11] af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.

[Paolo Abeni]

On Wed, 2024-06-26 at 14:47 -0700, Kuniyuki Iwashima wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> Date: Wed, 26 Jun 2024 23:10:40 +0200
> > On Wed, 2024-06-26 at 18:56 +0200, Paolo Abeni wrote:
> > > On Mon, 2024-06-24 at 18:36 -0700, Kuniyuki Iwashima wrote:
> > > > After consuming OOB data, recv() reading the preceding data must break at
> > > > the OOB skb regardless of MSG_PEEK.
> > > > 
> > > > Currently, MSG_PEEK does not stop recv() for AF_UNIX, and the behaviour is
> > > > not compliant with TCP.
> > > 
> > > I'm unsure we can change the MSG_PEEK behavior at this point: existing
> > > application(s?) could relay on that, regardless of how inconsistent
> > > such behavior is.
> > > 
> > > I think we need at least an explicit ack from Rao, as the main known
> > > user.
> > 
> > I see Rao stated that the unix OoB implementation was designed to mimic
> > the tcp one:
> > 
> > https://lore.kernel.org/netdev/c5f6abbe-de43-48b8-856a-36ded227e94f@oracle.com/
> > 
> > so the series should be ok.
> > 
> > Still given the size of the changes and the behavior change I'm
> > wondering if the series should target net-next instead.
> > That would offer some time cushion to deal with eventual regression.
> > WDYT?
> 
> The actual change is 37 LoC 

... excluding the other ~1200 LoC ;)

> and we recently have this kind of changes
> (30 LoC in total) in net.git.  The last two were merged in April and
> we have no user report so far.
> 
>   a6736a0addd6 af_unix: Read with MSG_PEEK loops if the first unread byte is OOB
>   22dd70eb2c3d af_unix: Don't peek OOB data without MSG_OOB.
>   283454c8a123 af_unix: Call manage_oob() for every skb in unix_stream_read_generic().

The last commit mentioned above actually sparked a bit of post merge
discussion, which is IMHO sub-optimal.

On the flip side, I think all the changes in this series make sense,
and the self-tests refactor and extension is largish, but very nice.

TL;DR: I'm going to apply this now.

Thanks,

Paolo

Re: [PATCH v1 net 03/11] af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.

[Rao Shoaib]

On 6/26/24 02:10, Paolo Abeni wrote:
> On Wed, 2024-06-26 at 18:56 +0200, Paolo Abeni wrote:
>> On Mon, 2024-06-24 at 18:36 -0700, Kuniyuki Iwashima wrote:
>>> After consuming OOB data, recv() reading the preceding data must break at
>>> the OOB skb regardless of MSG_PEEK.
>>>
>>> Currently, MSG_PEEK does not stop recv() for AF_UNIX, and the behaviour is
>>> not compliant with TCP.
>>
>> I'm unsure we can change the MSG_PEEK behavior at this point: existing
>> application(s?) could relay on that, regardless of how inconsistent
>> such behavior is.
>>
>> I think we need at least an explicit ack from Rao, as the main known
>> user.
> 
> I see Rao stated that the unix OoB implementation was designed to mimic
> the tcp one:
> 
> https://urldefense.com/v3/__https://lore.kernel.org/netdev/c5f6abbe-de43-48b8-856a-36ded227e94f@oracle.com/__;!!ACWV5N9M2RV99hQ!MxZ1Tdors9BCjgG4K-LeD_fvtJ0mQ6jgbR5CfGYIouxV7LbYRiKf7zCml6SKYN7OLG7LZnZHnBnVfyo$ 
> 
> so the series should be ok.

Yes.

Thanks,

Shoaib
> 
> Still given the size of the changes and the behavior change I'm
> wondering if the series should target net-next instead.
> That would offer some time cushion to deal with eventual regression.
> WDYT?
> 
> Thanks,
> 
> Paolo
> 

[PATCH v1 net 04/11] af_unix: Don't stop recv(MSG_DONTWAIT) if consumed OOB skb is at the head.

[Kuniyuki Iwashima]

Let's say a socket send()s "hello" with MSG_OOB and "world" without flags,

  >>> from socket import *
  >>> c1, c2 = socketpair(AF_UNIX)
  >>> c1.send(b'hello', MSG_OOB)
  5
  >>> c1.send(b'world')
  5

and its peer recv()s "hell" and "o".

  >>> c2.recv(10)
  b'hell'
  >>> c2.recv(1, MSG_OOB)
  b'o'

Now the consumed OOB skb stays at the head of recvq to return a correct
value for ioctl(SIOCATMARK), which is broken now and fixed by a later
patch.

Then, if peer issues recv() with MSG_DONTWAIT, manage_oob() returns NULL,
so recv() ends up with -EAGAIN.

  >>> c2.setblocking(False)  # This causes -EAGAIN even with available data
  >>> c2.recv(5)
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
  BlockingIOError: [Errno 11] Resource temporarily unavailable

However, next recv() will return the following available data, "world".

  >>> c2.recv(5)
  b'world'

When the consumed OOB skb is at the head of the queue, we need to fetch
the next skb to fix the weird behaviour.

Note that the issue does not happen without MSG_DONTWAIT because we can
retry after manage_oob().

This patch also adds a test case that covers the issue.

Without fix:

  #  RUN           msg_oob.no_peek.ex_oob_break ...
  # msg_oob.c:134:ex_oob_break:AF_UNIX :Resource temporarily unavailable
  # msg_oob.c:135:ex_oob_break:Expected:ld
  # msg_oob.c:137:ex_oob_break:Expected ret[0] (-1) == expected_len (2)
  # ex_oob_break: Test terminated by assertion
  #          FAIL  msg_oob.no_peek.ex_oob_break
  not ok 8 msg_oob.no_peek.ex_oob_break

With fix:

  #  RUN           msg_oob.no_peek.ex_oob_break ...
  #            OK  msg_oob.no_peek.ex_oob_break
  ok 8 msg_oob.no_peek.ex_oob_break

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c                            | 19 +++++++++++++++----
 tools/testing/selftests/net/af_unix/msg_oob.c | 11 +++++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 2eaecf9d78a4..b0b97f8d0d09 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2614,12 +2614,23 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 	struct unix_sock *u = unix_sk(sk);
 
 	if (!unix_skb_len(skb)) {
-		if (!(flags & MSG_PEEK)) {
-			skb_unlink(skb, &sk->sk_receive_queue);
-			consume_skb(skb);
+		struct sk_buff *unlinked_skb = NULL;
+
+		spin_lock(&sk->sk_receive_queue.lock);
+
+		if (copied) {
+			skb = NULL;
+		} else if (flags & MSG_PEEK) {
+			skb = skb_peek_next(skb, &sk->sk_receive_queue);
+		} else {
+			unlinked_skb = skb;
+			skb = skb_peek_next(skb, &sk->sk_receive_queue);
+			__skb_unlink(unlinked_skb, &sk->sk_receive_queue);
 		}
 
-		skb = NULL;
+		spin_unlock(&sk->sk_receive_queue.lock);
+
+		consume_skb(unlinked_skb);
 	} else {
 		struct sk_buff *unlinked_skb = NULL;
 
diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index de8d1fcde883..b5226ccec3ec 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -238,4 +238,15 @@ TEST_F(msg_oob, oob_break_drop)
 	recvpair("", -EINVAL, 1, MSG_OOB);
 }
 
+TEST_F(msg_oob, ex_oob_break)
+{
+	sendpair("hello", 5, MSG_OOB);
+	sendpair("wor", 3, MSG_OOB);
+	sendpair("ld", 2, 0);
+
+	recvpair("hellowo", 7, 10, 0);		/* Break at OOB but not at ex-OOB. */
+	recvpair("r", 1, 1, MSG_OOB);
+	recvpair("ld", 2, 2, 0);
+}
+
 TEST_HARNESS_MAIN
-- 
2.30.2

[PATCH v1 net 05/11] selftest: af_unix: Add non-TCP-compliant test cases in msg_oob.c.

[Kuniyuki Iwashima]

While testing, I found some weird behaviour on the TCP side as well.

For example, TCP drops the preceding OOB data when queueing a new
OOB data if the old OOB data is at the head of recvq.

  #  RUN           msg_oob.no_peek.ex_oob_drop ...
  # msg_oob.c:146:ex_oob_drop:AF_UNIX :x
  # msg_oob.c:147:ex_oob_drop:TCP     :Resource temporarily unavailable
  # msg_oob.c:146:ex_oob_drop:AF_UNIX :y
  # msg_oob.c:147:ex_oob_drop:TCP     :Invalid argument
  #            OK  msg_oob.no_peek.ex_oob_drop
  ok 9 msg_oob.no_peek.ex_oob_drop

  #  RUN           msg_oob.no_peek.ex_oob_drop_2 ...
  # msg_oob.c:146:ex_oob_drop_2:AF_UNIX :x
  # msg_oob.c:147:ex_oob_drop_2:TCP     :Resource temporarily unavailable
  #            OK  msg_oob.no_peek.ex_oob_drop_2
  ok 10 msg_oob.no_peek.ex_oob_drop_2

This patch allows AF_UNIX's MSG_OOB implementation to produce different
results from TCP when operations are guarded with tcp_incompliant{}.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 tools/testing/selftests/net/af_unix/msg_oob.c | 49 +++++++++++++++++--
 1 file changed, 44 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index b5226ccec3ec..46e92d06b0a3 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -19,6 +19,7 @@ FIXTURE(msg_oob)
 				 * 2: TCP sender
 				 * 3: TCP receiver
 				 */
+	bool tcp_compliant;
 };
 
 FIXTURE_VARIANT(msg_oob)
@@ -88,6 +89,8 @@ FIXTURE_SETUP(msg_oob)
 {
 	create_unix_socketpair(_metadata, self);
 	create_tcp_socketpair(_metadata, self);
+
+	self->tcp_compliant = true;
 }
 
 FIXTURE_TEARDOWN(msg_oob)
@@ -115,6 +118,7 @@ static void __recvpair(struct __test_metadata *_metadata,
 {
 	int i, ret[2], recv_errno[2], expected_errno = 0;
 	char recv_buf[2][BUF_SZ] = {};
+	bool printed = false;
 
 	ASSERT_GE(BUF_SZ, buf_len);
 
@@ -142,8 +146,12 @@ static void __recvpair(struct __test_metadata *_metadata,
 		TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
 		TH_LOG("TCP     :%s", ret[1] < 0 ? strerror(recv_errno[1]) : recv_buf[1]);
 
-		ASSERT_EQ(ret[0], ret[1]);
-		ASSERT_EQ(recv_errno[0], recv_errno[1]);
+		printed = true;
+
+		if (self->tcp_compliant) {
+			ASSERT_EQ(ret[0], ret[1]);
+			ASSERT_EQ(recv_errno[0], recv_errno[1]);
+		}
 	}
 
 	if (expected_len >= 0) {
@@ -159,10 +167,13 @@ static void __recvpair(struct __test_metadata *_metadata,
 
 		cmp = strncmp(recv_buf[0], recv_buf[1], expected_len);
 		if (cmp) {
-			TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
-			TH_LOG("TCP     :%s", ret[1] < 0 ? strerror(recv_errno[1]) : recv_buf[1]);
+			if (!printed) {
+				TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
+				TH_LOG("TCP     :%s", ret[1] < 0 ? strerror(recv_errno[1]) : recv_buf[1]);
+			}
 
-			ASSERT_EQ(cmp, 0);
+			if (self->tcp_compliant)
+				ASSERT_EQ(cmp, 0);
 		}
 	}
 }
@@ -180,6 +191,11 @@ static void __recvpair(struct __test_metadata *_metadata,
 			   expected_buf, expected_len, buf_len, flags);	\
 	} while (0)
 
+#define tcp_incompliant							\
+	for (self->tcp_compliant = false;				\
+	     self->tcp_compliant == false;				\
+	     self->tcp_compliant = true)
+
 TEST_F(msg_oob, non_oob)
 {
 	sendpair("x", 1, 0);
@@ -249,4 +265,27 @@ TEST_F(msg_oob, ex_oob_break)
 	recvpair("ld", 2, 2, 0);
 }
 
+TEST_F(msg_oob, ex_oob_drop)
+{
+	sendpair("x", 1, MSG_OOB);
+	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
+
+	tcp_incompliant {
+		recvpair("x", 1, 1, 0);		/* TCP drops "y" by passing through it. */
+		recvpair("y", 1, 1, MSG_OOB);	/* TCP returns -EINVAL. */
+	}
+}
+
+TEST_F(msg_oob, ex_oob_drop_2)
+{
+	sendpair("x", 1, MSG_OOB);
+	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
+
+	recvpair("y", 1, 1, MSG_OOB);
+
+	tcp_incompliant {
+		recvpair("x", 1, 1, 0);		/* TCP returns -EAGAIN. */
+	}
+}
+
 TEST_HARNESS_MAIN
-- 
2.30.2

[PATCH v1 net 06/11] af_unix: Don't stop recv() at consumed ex-OOB skb.

[Kuniyuki Iwashima]

Currently, recv() is stopped at a consumed OOB skb even if a new
OOB skb is queued and we can ignore the old OOB skb.

  >>> from socket import *
  >>> c1, c2 = socket(AF_UNIX, SOCK_STREAM)
  >>> c1.send(b'hellowor', MSG_OOB)
  8
  >>> c2.recv(1, MSG_OOB)  # consume OOB data stays at middle of recvq.
  b'r'
  >>> c1.send(b'ld', MSG_OOB)
  2
  >>> c2.recv(10)          # recv() stops at the old consumed OOB
  b'hellowo'               # should be 'hellowol'

manage_oob() should not stop recv() at the old consumed OOB skb if
there is a new OOB data queued.

Note that TCP behaviour is apparently wrong in this test case because
we can recv() the same OOB data twice.

Without fix:

  #  RUN           msg_oob.no_peek.ex_oob_ahead_break ...
  # msg_oob.c:138:ex_oob_ahead_break:AF_UNIX :hellowo
  # msg_oob.c:139:ex_oob_ahead_break:Expected:hellowol
  # msg_oob.c:141:ex_oob_ahead_break:Expected ret[0] (7) == expected_len (8)
  # ex_oob_ahead_break: Test terminated by assertion
  #          FAIL  msg_oob.no_peek.ex_oob_ahead_break
  not ok 11 msg_oob.no_peek.ex_oob_ahead_break

With fix:

  #  RUN           msg_oob.no_peek.ex_oob_ahead_break ...
  # msg_oob.c:146:ex_oob_ahead_break:AF_UNIX :hellowol
  # msg_oob.c:147:ex_oob_ahead_break:TCP     :helloworl
  #            OK  msg_oob.no_peek.ex_oob_ahead_break
  ok 11 msg_oob.no_peek.ex_oob_ahead_break

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c                            |  2 +-
 tools/testing/selftests/net/af_unix/msg_oob.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index b0b97f8d0d09..07f5eaa04b5b 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2618,7 +2618,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 
 		spin_lock(&sk->sk_receive_queue.lock);
 
-		if (copied) {
+		if (copied && (!u->oob_skb || skb == u->oob_skb)) {
 			skb = NULL;
 		} else if (flags & MSG_PEEK) {
 			skb = skb_peek_next(skb, &sk->sk_receive_queue);
diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 46e92d06b0a3..acf4bd0afe17 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -288,4 +288,20 @@ TEST_F(msg_oob, ex_oob_drop_2)
 	}
 }
 
+TEST_F(msg_oob, ex_oob_ahead_break)
+{
+	sendpair("hello", 5, MSG_OOB);
+	sendpair("wor", 3, MSG_OOB);
+
+	recvpair("r", 1, 1, MSG_OOB);
+
+	sendpair("ld", 2, MSG_OOB);
+
+	tcp_incompliant {
+		recvpair("hellowol", 8, 10, 0);	/* TCP recv()s "helloworl", why "r" ?? */
+	}
+
+	recvpair("d", 1, 1, MSG_OOB);
+}
+
 TEST_HARNESS_MAIN
-- 
2.30.2

[PATCH v1 net 07/11] selftest: af_unix: Add SO_OOBINLINE test cases in msg_oob.c

[Kuniyuki Iwashima]

When SO_OOBINLINE is enabled on a socket, MSG_OOB can be recv()ed
without MSG_OOB flag, and ioctl(SIOCATMARK) will behaves differently.

This patch adds some test cases for SO_OOBINLINE.

Note the new test cases found two bugs in TCP.

  1) After reading OOB data with non-inline mode, we can re-read
     the data by setting SO_OOBINLINE.

  #  RUN           msg_oob.no_peek.inline_oob_ahead_break ...
  # msg_oob.c:146:inline_oob_ahead_break:AF_UNIX :world
  # msg_oob.c:147:inline_oob_ahead_break:TCP     :oworld
  #            OK  msg_oob.no_peek.inline_oob_ahead_break
  ok 14 msg_oob.no_peek.inline_oob_ahead_break

  2) The head OOB data is dropped if SO_OOBINLINE is disabled
     if a new OOB data is queued.

  #  RUN           msg_oob.no_peek.inline_ex_oob_drop ...
  # msg_oob.c:171:inline_ex_oob_drop:AF_UNIX :x
  # msg_oob.c:172:inline_ex_oob_drop:TCP     :y
  # msg_oob.c:146:inline_ex_oob_drop:AF_UNIX :y
  # msg_oob.c:147:inline_ex_oob_drop:TCP     :Resource temporarily unavailable
  #            OK  msg_oob.no_peek.inline_ex_oob_drop
  ok 17 msg_oob.no_peek.inline_ex_oob_drop

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 tools/testing/selftests/net/af_unix/msg_oob.c | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index acf4bd0afe17..62361b5e98c3 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -178,6 +178,20 @@ static void __recvpair(struct __test_metadata *_metadata,
 	}
 }
 
+static void __setinlinepair(struct __test_metadata *_metadata,
+			    FIXTURE_DATA(msg_oob) *self)
+{
+	int i, oob_inline = 1;
+
+	for (i = 0; i < 2; i++) {
+		int ret;
+
+		ret = setsockopt(self->fd[i * 2 + 1], SOL_SOCKET, SO_OOBINLINE,
+				 &oob_inline, sizeof(oob_inline));
+		ASSERT_EQ(ret, 0);
+	}
+}
+
 #define sendpair(buf, len, flags)					\
 	__sendpair(_metadata, self, buf, len, flags)
 
@@ -191,6 +205,9 @@ static void __recvpair(struct __test_metadata *_metadata,
 			   expected_buf, expected_len, buf_len, flags);	\
 	} while (0)
 
+#define setinlinepair()							\
+	__setinlinepair(_metadata, self)
+
 #define tcp_incompliant							\
 	for (self->tcp_compliant = false;				\
 	     self->tcp_compliant == false;				\
@@ -304,4 +321,78 @@ TEST_F(msg_oob, ex_oob_ahead_break)
 	recvpair("d", 1, 1, MSG_OOB);
 }
 
+TEST_F(msg_oob, inline_oob)
+{
+	setinlinepair();
+
+	sendpair("x", 1, MSG_OOB);
+
+	recvpair("", -EINVAL, 1, MSG_OOB);
+	recvpair("x", 1, 1, 0);
+}
+
+TEST_F(msg_oob, inline_oob_break)
+{
+	setinlinepair();
+
+	sendpair("hello", 5, MSG_OOB);
+
+	recvpair("", -EINVAL, 1, MSG_OOB);
+	recvpair("hell", 4, 5, 0);		/* Break at OOB but not at ex-OOB. */
+	recvpair("o", 1, 1, 0);
+}
+
+TEST_F(msg_oob, inline_oob_ahead_break)
+{
+	sendpair("hello", 5, MSG_OOB);
+	sendpair("world", 5, 0);
+
+	recvpair("o", 1, 1, MSG_OOB);
+
+	setinlinepair();
+
+	recvpair("hell", 4, 9, 0);		/* Break at OOB even with enough buffer. */
+
+	tcp_incompliant {
+		recvpair("world", 5, 6, 0);	/* TCP recv()s "oworld", ... "o" ??? */
+	}
+}
+
+TEST_F(msg_oob, inline_ex_oob_break)
+{
+	sendpair("hello", 5, MSG_OOB);
+	sendpair("wor", 3, MSG_OOB);
+	sendpair("ld", 2, 0);
+
+	setinlinepair();
+
+	recvpair("hellowo", 7, 10, 0);		/* Break at OOB but not at ex-OOB. */
+	recvpair("rld", 3, 3, 0);
+}
+
+TEST_F(msg_oob, inline_ex_oob_no_drop)
+{
+	sendpair("x", 1, MSG_OOB);
+
+	setinlinepair();
+
+	sendpair("y", 1, MSG_OOB);		/* TCP does NOT drops "x" at this moment. */
+
+	recvpair("x", 1, 1, 0);
+	recvpair("y", 1, 1, 0);
+}
+
+TEST_F(msg_oob, inline_ex_oob_drop)
+{
+	sendpair("x", 1, MSG_OOB);
+	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
+
+	setinlinepair();
+
+	tcp_incompliant {
+		recvpair("x", 1, 1, 0);		/* TCP recv()s "y". */
+		recvpair("y", 1, 1, 0);		/* TCP returns -EAGAIN. */
+	}
+}
+
 TEST_HARNESS_MAIN
-- 
2.30.2

[PATCH v1 net 08/11] selftest: af_unix: Check SIGURG after every send() in msg_oob.c

[Kuniyuki Iwashima]

When data is sent with MSG_OOB, SIGURG is sent to a process if the
receiver socket has set its owner to the process by ioctl(FIOSETOWN)
or fcntl(F_SETOWN).

This patch adds SIGURG check after every send(MSG_OOB) call.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 tools/testing/selftests/net/af_unix/msg_oob.c | 51 ++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 62361b5e98c3..123dee0b6739 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -6,6 +6,8 @@
 #include <unistd.h>
 
 #include <netinet/in.h>
+#include <sys/ioctl.h>
+#include <sys/signalfd.h>
 #include <sys/socket.h>
 
 #include "../../kselftest_harness.h"
@@ -19,6 +21,7 @@ FIXTURE(msg_oob)
 				 * 2: TCP sender
 				 * 3: TCP receiver
 				 */
+	int signal_fd;
 	bool tcp_compliant;
 };
 
@@ -77,6 +80,35 @@ static void create_tcp_socketpair(struct __test_metadata *_metadata,
 	ASSERT_EQ(ret, 0);
 }
 
+static void setup_sigurg(struct __test_metadata *_metadata,
+			 FIXTURE_DATA(msg_oob) *self)
+{
+	struct signalfd_siginfo siginfo;
+	int pid = getpid();
+	sigset_t mask;
+	int i, ret;
+
+	for (i = 0; i < 2; i++) {
+		ret = ioctl(self->fd[i * 2 + 1], FIOSETOWN, &pid);
+		ASSERT_EQ(ret, 0);
+	}
+
+	ret = sigemptyset(&mask);
+	ASSERT_EQ(ret, 0);
+
+	ret = sigaddset(&mask, SIGURG);
+	ASSERT_EQ(ret, 0);
+
+	ret = sigprocmask(SIG_BLOCK, &mask, NULL);
+	ASSERT_EQ(ret, 0);
+
+	self->signal_fd = signalfd(-1, &mask, SFD_NONBLOCK);
+	ASSERT_GE(self->signal_fd, 0);
+
+	ret = read(self->signal_fd, &siginfo, sizeof(siginfo));
+	ASSERT_EQ(ret, -1);
+}
+
 static void close_sockets(FIXTURE_DATA(msg_oob) *self)
 {
 	int i;
@@ -90,6 +122,8 @@ FIXTURE_SETUP(msg_oob)
 	create_unix_socketpair(_metadata, self);
 	create_tcp_socketpair(_metadata, self);
 
+	setup_sigurg(_metadata, self);
+
 	self->tcp_compliant = true;
 }
 
@@ -104,9 +138,24 @@ static void __sendpair(struct __test_metadata *_metadata,
 {
 	int i, ret[2];
 
-	for (i = 0; i < 2; i++)
+	for (i = 0; i < 2; i++) {
+		struct signalfd_siginfo siginfo = {};
+		int bytes;
+
 		ret[i] = send(self->fd[i * 2], buf, len, flags);
 
+		bytes = read(self->signal_fd, &siginfo, sizeof(siginfo));
+
+		if (flags & MSG_OOB) {
+			ASSERT_EQ(bytes, sizeof(siginfo));
+			ASSERT_EQ(siginfo.ssi_signo, SIGURG);
+
+			bytes = read(self->signal_fd, &siginfo, sizeof(siginfo));
+		}
+
+		ASSERT_EQ(bytes, -1);
+	}
+
 	ASSERT_EQ(ret[0], len);
 	ASSERT_EQ(ret[0], ret[1]);
 }
-- 
2.30.2

[PATCH v1 net 09/11] selftest: af_unix: Check EPOLLPRI after every send()/recv() in msg_oob.c

[Kuniyuki Iwashima]

When OOB data is in recvq, we can detect it with epoll by checking
EPOLLPRI.

This patch add checks for EPOLLPRI after every send() and recv() in
all test cases.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 tools/testing/selftests/net/af_unix/msg_oob.c | 147 ++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 123dee0b6739..28b09b36a2f1 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -6,6 +6,7 @@
 #include <unistd.h>
 
 #include <netinet/in.h>
+#include <sys/epoll.h>
 #include <sys/ioctl.h>
 #include <sys/signalfd.h>
 #include <sys/socket.h>
@@ -22,6 +23,9 @@ FIXTURE(msg_oob)
 				 * 3: TCP receiver
 				 */
 	int signal_fd;
+	int epoll_fd[2];	/* 0: AF_UNIX receiver
+				 * 1: TCP receiver
+				 */
 	bool tcp_compliant;
 };
 
@@ -109,6 +113,25 @@ static void setup_sigurg(struct __test_metadata *_metadata,
 	ASSERT_EQ(ret, -1);
 }
 
+static void setup_epollpri(struct __test_metadata *_metadata,
+			   FIXTURE_DATA(msg_oob) *self)
+{
+	struct epoll_event event = {
+		.events = EPOLLPRI,
+	};
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		int ret;
+
+		self->epoll_fd[i] = epoll_create1(0);
+		ASSERT_GE(self->epoll_fd[i], 0);
+
+		ret = epoll_ctl(self->epoll_fd[i], EPOLL_CTL_ADD, self->fd[i * 2 + 1], &event);
+		ASSERT_EQ(ret, 0);
+	}
+}
+
 static void close_sockets(FIXTURE_DATA(msg_oob) *self)
 {
 	int i;
@@ -123,6 +146,7 @@ FIXTURE_SETUP(msg_oob)
 	create_tcp_socketpair(_metadata, self);
 
 	setup_sigurg(_metadata, self);
+	setup_epollpri(_metadata, self);
 
 	self->tcp_compliant = true;
 }
@@ -132,6 +156,29 @@ FIXTURE_TEARDOWN(msg_oob)
 	close_sockets(self);
 }
 
+static void __epollpair(struct __test_metadata *_metadata,
+			FIXTURE_DATA(msg_oob) *self,
+			bool oob_remaining)
+{
+	struct epoll_event event[2] = {};
+	int i, ret[2];
+
+	for (i = 0; i < 2; i++)
+		ret[i] = epoll_wait(self->epoll_fd[i], &event[i], 1, 0);
+
+	ASSERT_EQ(ret[0], oob_remaining);
+
+	if (self->tcp_compliant)
+		ASSERT_EQ(ret[0], ret[1]);
+
+	if (oob_remaining) {
+		ASSERT_EQ(event[0].events, EPOLLPRI);
+
+		if (self->tcp_compliant)
+			ASSERT_EQ(event[0].events, event[1].events);
+	}
+}
+
 static void __sendpair(struct __test_metadata *_metadata,
 		       FIXTURE_DATA(msg_oob) *self,
 		       const void *buf, size_t len, int flags)
@@ -254,6 +301,9 @@ static void __setinlinepair(struct __test_metadata *_metadata,
 			   expected_buf, expected_len, buf_len, flags);	\
 	} while (0)
 
+#define epollpair(oob_remaining)					\
+	__epollpair(_metadata, self, oob_remaining)
+
 #define setinlinepair()							\
 	__setinlinepair(_metadata, self)
 
@@ -265,109 +315,170 @@ static void __setinlinepair(struct __test_metadata *_metadata,
 TEST_F(msg_oob, non_oob)
 {
 	sendpair("x", 1, 0);
+	epollpair(false);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, oob)
 {
 	sendpair("x", 1, MSG_OOB);
+	epollpair(true);
 
 	recvpair("x", 1, 1, MSG_OOB);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, oob_drop)
 {
 	sendpair("x", 1, MSG_OOB);
+	epollpair(true);
 
 	recvpair("", -EAGAIN, 1, 0);		/* Drop OOB. */
+	epollpair(false);
+
 	recvpair("", -EINVAL, 1, MSG_OOB);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, oob_ahead)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
 
 	recvpair("o", 1, 1, MSG_OOB);
+	epollpair(false);
+
 	recvpair("hell", 4, 4, 0);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, oob_break)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
 
 	recvpair("hell", 4, 5, 0);		/* Break at OOB even with enough buffer. */
+	epollpair(true);
+
 	recvpair("o", 1, 1, MSG_OOB);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, oob_ahead_break)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+
 	sendpair("world", 5, 0);
+	epollpair(true);
 
 	recvpair("o", 1, 1, MSG_OOB);
+	epollpair(false);
+
 	recvpair("hell", 4, 9, 0);		/* Break at OOB even after it's recv()ed. */
+	epollpair(false);
+
 	recvpair("world", 5, 5, 0);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, oob_break_drop)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+
 	sendpair("world", 5, 0);
+	epollpair(true);
 
 	recvpair("hell", 4, 10, 0);		/* Break at OOB even with enough buffer. */
+	epollpair(true);
+
 	recvpair("world", 5, 10, 0);		/* Drop OOB and recv() the next skb. */
+	epollpair(false);
+
 	recvpair("", -EINVAL, 1, MSG_OOB);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, ex_oob_break)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+
 	sendpair("wor", 3, MSG_OOB);
+	epollpair(true);
+
 	sendpair("ld", 2, 0);
+	epollpair(true);
 
 	recvpair("hellowo", 7, 10, 0);		/* Break at OOB but not at ex-OOB. */
+	epollpair(true);
+
 	recvpair("r", 1, 1, MSG_OOB);
+	epollpair(false);
+
 	recvpair("ld", 2, 2, 0);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, ex_oob_drop)
 {
 	sendpair("x", 1, MSG_OOB);
+	epollpair(true);
+
 	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
+	epollpair(true);
 
 	tcp_incompliant {
 		recvpair("x", 1, 1, 0);		/* TCP drops "y" by passing through it. */
+		epollpair(true);
+
 		recvpair("y", 1, 1, MSG_OOB);	/* TCP returns -EINVAL. */
+		epollpair(false);
 	}
 }
 
 TEST_F(msg_oob, ex_oob_drop_2)
 {
 	sendpair("x", 1, MSG_OOB);
+	epollpair(true);
+
 	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
+	epollpair(true);
 
 	recvpair("y", 1, 1, MSG_OOB);
+	epollpair(false);
 
 	tcp_incompliant {
 		recvpair("x", 1, 1, 0);		/* TCP returns -EAGAIN. */
+		epollpair(false);
 	}
 }
 
 TEST_F(msg_oob, ex_oob_ahead_break)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+
 	sendpair("wor", 3, MSG_OOB);
+	epollpair(true);
 
 	recvpair("r", 1, 1, MSG_OOB);
+	epollpair(false);
 
 	sendpair("ld", 2, MSG_OOB);
+	epollpair(true);
 
 	tcp_incompliant {
 		recvpair("hellowol", 8, 10, 0);	/* TCP recv()s "helloworl", why "r" ?? */
 	}
 
+	epollpair(true);
+
 	recvpair("d", 1, 1, MSG_OOB);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, inline_oob)
@@ -375,9 +486,13 @@ TEST_F(msg_oob, inline_oob)
 	setinlinepair();
 
 	sendpair("x", 1, MSG_OOB);
+	epollpair(true);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
+	epollpair(true);
+
 	recvpair("x", 1, 1, 0);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, inline_oob_break)
@@ -385,62 +500,94 @@ TEST_F(msg_oob, inline_oob_break)
 	setinlinepair();
 
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
+	epollpair(true);
+
 	recvpair("hell", 4, 5, 0);		/* Break at OOB but not at ex-OOB. */
+	epollpair(true);
+
 	recvpair("o", 1, 1, 0);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, inline_oob_ahead_break)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+
 	sendpair("world", 5, 0);
+	epollpair(true);
 
 	recvpair("o", 1, 1, MSG_OOB);
+	epollpair(false);
 
 	setinlinepair();
 
 	recvpair("hell", 4, 9, 0);		/* Break at OOB even with enough buffer. */
+	epollpair(false);
 
 	tcp_incompliant {
 		recvpair("world", 5, 6, 0);	/* TCP recv()s "oworld", ... "o" ??? */
 	}
+
+	epollpair(false);
 }
 
 TEST_F(msg_oob, inline_ex_oob_break)
 {
 	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+
 	sendpair("wor", 3, MSG_OOB);
+	epollpair(true);
+
 	sendpair("ld", 2, 0);
+	epollpair(true);
 
 	setinlinepair();
 
 	recvpair("hellowo", 7, 10, 0);		/* Break at OOB but not at ex-OOB. */
+	epollpair(true);
+
 	recvpair("rld", 3, 3, 0);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, inline_ex_oob_no_drop)
 {
 	sendpair("x", 1, MSG_OOB);
+	epollpair(true);
 
 	setinlinepair();
 
 	sendpair("y", 1, MSG_OOB);		/* TCP does NOT drops "x" at this moment. */
+	epollpair(true);
 
 	recvpair("x", 1, 1, 0);
+	epollpair(true);
+
 	recvpair("y", 1, 1, 0);
+	epollpair(false);
 }
 
 TEST_F(msg_oob, inline_ex_oob_drop)
 {
 	sendpair("x", 1, MSG_OOB);
+	epollpair(true);
+
 	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
+	epollpair(true);
 
 	setinlinepair();
 
 	tcp_incompliant {
 		recvpair("x", 1, 1, 0);		/* TCP recv()s "y". */
+		epollpair(true);
+
 		recvpair("y", 1, 1, 0);		/* TCP returns -EAGAIN. */
+		epollpair(false);
 	}
 }
 
-- 
2.30.2

[PATCH v1 net 10/11] af_unix: Fix wrong ioctl(SIOCATMARK) when consumed OOB skb is at the head.

[Kuniyuki Iwashima]

Even if OOB data is recv()ed, ioctl(SIOCATMARK) must return 1 when the
OOB skb is at the head of the receive queue and no new OOB data is queued.

Without fix:

  #  RUN           msg_oob.no_peek.oob ...
  # msg_oob.c:305:oob:Expected answ[0] (0) == oob_head (1)
  # oob: Test terminated by assertion
  #          FAIL  msg_oob.no_peek.oob
  not ok 2 msg_oob.no_peek.oob

With fix:

  #  RUN           msg_oob.no_peek.oob ...
  #            OK  msg_oob.no_peek.oob
  ok 2 msg_oob.no_peek.oob

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c                            | 15 +++-
 tools/testing/selftests/net/af_unix/msg_oob.c | 68 +++++++++++++++++++
 2 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 07f5eaa04b5b..142f56770b77 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -3107,12 +3107,23 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
 	case SIOCATMARK:
 		{
+			struct unix_sock *u = unix_sk(sk);
 			struct sk_buff *skb;
 			int answ = 0;
 
+			mutex_lock(&u->iolock);
+
 			skb = skb_peek(&sk->sk_receive_queue);
-			if (skb && skb == READ_ONCE(unix_sk(sk)->oob_skb))
-				answ = 1;
+			if (skb) {
+				struct sk_buff *oob_skb = READ_ONCE(u->oob_skb);
+
+				if (skb == oob_skb ||
+				    (!oob_skb && !unix_skb_len(skb)))
+					answ = 1;
+			}
+
+			mutex_unlock(&u->iolock);
+
 			err = put_user(answ, (int __user *)arg);
 		}
 		break;
diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 28b09b36a2f1..2d0024329437 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -288,6 +288,26 @@ static void __setinlinepair(struct __test_metadata *_metadata,
 	}
 }
 
+static void __siocatmarkpair(struct __test_metadata *_metadata,
+			     FIXTURE_DATA(msg_oob) *self,
+			     bool oob_head)
+{
+	int answ[2] = {};
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		int ret;
+
+		ret = ioctl(self->fd[i * 2 + 1], SIOCATMARK, &answ[i]);
+		ASSERT_EQ(ret, 0);
+	}
+
+	ASSERT_EQ(answ[0], oob_head);
+
+	if (self->tcp_compliant)
+		ASSERT_EQ(answ[0], answ[1]);
+}
+
 #define sendpair(buf, len, flags)					\
 	__sendpair(_metadata, self, buf, len, flags)
 
@@ -304,6 +324,9 @@ static void __setinlinepair(struct __test_metadata *_metadata,
 #define epollpair(oob_remaining)					\
 	__epollpair(_metadata, self, oob_remaining)
 
+#define siocatmarkpair(oob_head)					\
+	__siocatmarkpair(_metadata, self, oob_head)
+
 #define setinlinepair()							\
 	__setinlinepair(_metadata, self)
 
@@ -325,9 +348,11 @@ TEST_F(msg_oob, oob)
 {
 	sendpair("x", 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("x", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(true);
 }
 
 TEST_F(msg_oob, oob_drop)
@@ -481,18 +506,40 @@ TEST_F(msg_oob, ex_oob_ahead_break)
 	epollpair(false);
 }
 
+TEST_F(msg_oob, ex_oob_siocatmark)
+{
+	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+	siocatmarkpair(false);
+
+	recvpair("o", 1, 1, MSG_OOB);
+	epollpair(false);
+	siocatmarkpair(false);
+
+	sendpair("world", 5, MSG_OOB);
+	epollpair(true);
+	siocatmarkpair(false);
+
+	recvpair("hell", 4, 4, 0);		/* Intentionally stop at ex-OOB. */
+	epollpair(true);
+	siocatmarkpair(false);
+}
+
 TEST_F(msg_oob, inline_oob)
 {
 	setinlinepair();
 
 	sendpair("x", 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("x", 1, 1, 0);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, inline_oob_break)
@@ -591,4 +638,25 @@ TEST_F(msg_oob, inline_ex_oob_drop)
 	}
 }
 
+TEST_F(msg_oob, inline_ex_oob_siocatmark)
+{
+	sendpair("hello", 5, MSG_OOB);
+	epollpair(true);
+	siocatmarkpair(false);
+
+	recvpair("o", 1, 1, MSG_OOB);
+	epollpair(false);
+	siocatmarkpair(false);
+
+	setinlinepair();
+
+	sendpair("world", 5, MSG_OOB);
+	epollpair(true);
+	siocatmarkpair(false);
+
+	recvpair("hell", 4, 4, 0);		/* Intentionally stop at ex-OOB. */
+	epollpair(true);
+	siocatmarkpair(false);
+}
+
 TEST_HARNESS_MAIN
-- 
2.30.2

[PATCH v1 net 11/11] selftest: af_unix: Check SIOCATMARK after every send()/recv() in msg_oob.c.

[Kuniyuki Iwashima]

To catch regression, let's check ioctl(SIOCATMARK) after every
send() and recv() calls.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 tools/testing/selftests/net/af_unix/msg_oob.c | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 2d0024329437..16d0c172eaeb 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -339,9 +339,11 @@ TEST_F(msg_oob, non_oob)
 {
 	sendpair("x", 1, 0);
 	epollpair(false);
+	siocatmarkpair(false);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, oob)
@@ -359,109 +361,142 @@ TEST_F(msg_oob, oob_drop)
 {
 	sendpair("x", 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("", -EAGAIN, 1, 0);		/* Drop OOB. */
 	epollpair(false);
+	siocatmarkpair(false);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, oob_ahead)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("o", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(false);
 
 	recvpair("hell", 4, 4, 0);
 	epollpair(false);
+	siocatmarkpair(true);
 }
 
 TEST_F(msg_oob, oob_break)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("hell", 4, 5, 0);		/* Break at OOB even with enough buffer. */
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("o", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(true);
+
+	recvpair("", -EAGAIN, 1, 0);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, oob_ahead_break)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("world", 5, 0);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("o", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(false);
 
 	recvpair("hell", 4, 9, 0);		/* Break at OOB even after it's recv()ed. */
 	epollpair(false);
+	siocatmarkpair(true);
 
 	recvpair("world", 5, 5, 0);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, oob_break_drop)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("world", 5, 0);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("hell", 4, 10, 0);		/* Break at OOB even with enough buffer. */
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("world", 5, 10, 0);		/* Drop OOB and recv() the next skb. */
 	epollpair(false);
+	siocatmarkpair(false);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, ex_oob_break)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("wor", 3, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("ld", 2, 0);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("hellowo", 7, 10, 0);		/* Break at OOB but not at ex-OOB. */
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("r", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(true);
 
 	recvpair("ld", 2, 2, 0);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, ex_oob_drop)
 {
 	sendpair("x", 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
 	epollpair(true);
 
 	tcp_incompliant {
+		siocatmarkpair(false);
+
 		recvpair("x", 1, 1, 0);		/* TCP drops "y" by passing through it. */
 		epollpair(true);
+		siocatmarkpair(true);
 
 		recvpair("y", 1, 1, MSG_OOB);	/* TCP returns -EINVAL. */
 		epollpair(false);
+		siocatmarkpair(true);
 	}
 }
 
@@ -469,16 +504,24 @@ TEST_F(msg_oob, ex_oob_drop_2)
 {
 	sendpair("x", 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
 	epollpair(true);
 
+	tcp_incompliant {
+		siocatmarkpair(false);
+	}
+
 	recvpair("y", 1, 1, MSG_OOB);
 	epollpair(false);
 
 	tcp_incompliant {
+		siocatmarkpair(false);
+
 		recvpair("x", 1, 1, 0);		/* TCP returns -EAGAIN. */
 		epollpair(false);
+		siocatmarkpair(true);
 	}
 }
 
@@ -486,24 +529,30 @@ TEST_F(msg_oob, ex_oob_ahead_break)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("wor", 3, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("r", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(false);
 
 	sendpair("ld", 2, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	tcp_incompliant {
 		recvpair("hellowol", 8, 10, 0);	/* TCP recv()s "helloworl", why "r" ?? */
 	}
 
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("d", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(true);
 }
 
 TEST_F(msg_oob, ex_oob_siocatmark)
@@ -548,81 +597,100 @@ TEST_F(msg_oob, inline_oob_break)
 
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("", -EINVAL, 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("hell", 4, 5, 0);		/* Break at OOB but not at ex-OOB. */
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("o", 1, 1, 0);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, inline_oob_ahead_break)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("world", 5, 0);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("o", 1, 1, MSG_OOB);
 	epollpair(false);
+	siocatmarkpair(false);
 
 	setinlinepair();
 
 	recvpair("hell", 4, 9, 0);		/* Break at OOB even with enough buffer. */
 	epollpair(false);
+	siocatmarkpair(true);
 
 	tcp_incompliant {
 		recvpair("world", 5, 6, 0);	/* TCP recv()s "oworld", ... "o" ??? */
 	}
 
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, inline_ex_oob_break)
 {
 	sendpair("hello", 5, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("wor", 3, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	sendpair("ld", 2, 0);
 	epollpair(true);
+	siocatmarkpair(false);
 
 	setinlinepair();
 
 	recvpair("hellowo", 7, 10, 0);		/* Break at OOB but not at ex-OOB. */
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("rld", 3, 3, 0);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, inline_ex_oob_no_drop)
 {
 	sendpair("x", 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	setinlinepair();
 
 	sendpair("y", 1, MSG_OOB);		/* TCP does NOT drops "x" at this moment. */
 	epollpair(true);
+	siocatmarkpair(false);
 
 	recvpair("x", 1, 1, 0);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	recvpair("y", 1, 1, 0);
 	epollpair(false);
+	siocatmarkpair(false);
 }
 
 TEST_F(msg_oob, inline_ex_oob_drop)
 {
 	sendpair("x", 1, MSG_OOB);
 	epollpair(true);
+	siocatmarkpair(true);
 
 	sendpair("y", 1, MSG_OOB);		/* TCP drops "x" at this moment. */
 	epollpair(true);
@@ -630,11 +698,15 @@ TEST_F(msg_oob, inline_ex_oob_drop)
 	setinlinepair();
 
 	tcp_incompliant {
+		siocatmarkpair(false);
+
 		recvpair("x", 1, 1, 0);		/* TCP recv()s "y". */
 		epollpair(true);
+		siocatmarkpair(true);
 
 		recvpair("y", 1, 1, 0);		/* TCP returns -EAGAIN. */
 		epollpair(false);
+		siocatmarkpair(false);
 	}
 }
 
-- 
2.30.2

Re: [PATCH v1 net 00/11] af_unix: Fix bunch of MSG_OOB bugs and add new tests.

[Jakub Kicinski]

On Mon, 24 Jun 2024 18:36:34 -0700 Kuniyuki Iwashima wrote:
> This series rewrites the selftest for AF_UNIX MSG_OOB and fixes
> bunch of bugs that AF_UNIX behaves differently compared to TCP.

I like pairing the fix with the selftest, but at the same time
"let's rewrite the selftest first" gives me pause. We have 40 LoC
of actual changes here and 1000 LoC of test churn.

I guess we'll find out on Thursday if we went too far :)

>  net/unix/af_unix.c                            |  37 +-
>  tools/testing/selftests/net/.gitignore        |   1 -
>  tools/testing/selftests/net/af_unix/Makefile  |   2 +-
>  tools/testing/selftests/net/af_unix/msg_oob.c | 734 ++++++++++++++++++
>  .../selftests/net/af_unix/test_unix_oob.c     | 436 -----------
>  5 files changed, 766 insertions(+), 444 deletions(-)
>  create mode 100644 tools/testing/selftests/net/af_unix/msg_oob.c
>  delete mode 100644 tools/testing/selftests/net/af_unix/test_unix_oob.c

Re: [PATCH v1 net 00/11] af_unix: Fix bunch of MSG_OOB bugs and add new tests.

[Kuniyuki Iwashima]

From: Jakub Kicinski <kuba@kernel.org>
Date: Tue, 25 Jun 2024 17:43:18 -0700
> On Mon, 24 Jun 2024 18:36:34 -0700 Kuniyuki Iwashima wrote:
> > This series rewrites the selftest for AF_UNIX MSG_OOB and fixes
> > bunch of bugs that AF_UNIX behaves differently compared to TCP.
> 
> I like pairing the fix with the selftest, but at the same time
> "let's rewrite the selftest first" gives me pause. We have 40 LoC
> of actual changes here and 1000 LoC of test churn.
> 
> I guess we'll find out on Thursday if we went too far :)

I hope it's worth for finding 4 AF_UNIX bugs and 2 ancient TCP ones,
let's see 🤞


> 
> >  net/unix/af_unix.c                            |  37 +-
> >  tools/testing/selftests/net/.gitignore        |   1 -
> >  tools/testing/selftests/net/af_unix/Makefile  |   2 +-
> >  tools/testing/selftests/net/af_unix/msg_oob.c | 734 ++++++++++++++++++
> >  .../selftests/net/af_unix/test_unix_oob.c     | 436 -----------
> >  5 files changed, 766 insertions(+), 444 deletions(-)
> >  create mode 100644 tools/testing/selftests/net/af_unix/msg_oob.c
> >  delete mode 100644 tools/testing/selftests/net/af_unix/test_unix_oob.c

Re: [PATCH v1 net 00/11] af_unix: Fix bunch of MSG_OOB bugs and add new tests.

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 24 Jun 2024 18:36:34 -0700 you wrote:
> This series rewrites the selftest for AF_UNIX MSG_OOB and fixes
> bunch of bugs that AF_UNIX behaves differently compared to TCP.
> 
> Note that the test discovered few more bugs in TCP side, which
> will be fixed in another series.
> 
> 
> [...]

Here is the summary with links:
  - [v1,net,01/11] selftest: af_unix: Remove test_unix_oob.c.
    https://git.kernel.org/netdev/net/c/7d139181a891
  - [v1,net,02/11] selftest: af_unix: Add msg_oob.c.
    https://git.kernel.org/netdev/net/c/d098d77232c3
  - [v1,net,03/11] af_unix: Stop recv(MSG_PEEK) at consumed OOB skb.
    https://git.kernel.org/netdev/net/c/b94038d841a9
  - [v1,net,04/11] af_unix: Don't stop recv(MSG_DONTWAIT) if consumed OOB skb is at the head.
    https://git.kernel.org/netdev/net/c/93c99f21db36
  - [v1,net,05/11] selftest: af_unix: Add non-TCP-compliant test cases in msg_oob.c.
    https://git.kernel.org/netdev/net/c/f5ea0768a255
  - [v1,net,06/11] af_unix: Don't stop recv() at consumed ex-OOB skb.
    https://git.kernel.org/netdev/net/c/36893ef0b661
  - [v1,net,07/11] selftest: af_unix: Add SO_OOBINLINE test cases in msg_oob.c
    https://git.kernel.org/netdev/net/c/436352e8e57e
  - [v1,net,08/11] selftest: af_unix: Check SIGURG after every send() in msg_oob.c
    https://git.kernel.org/netdev/net/c/d02689e6860d
  - [v1,net,09/11] selftest: af_unix: Check EPOLLPRI after every send()/recv() in msg_oob.c
    https://git.kernel.org/netdev/net/c/48a998373090
  - [v1,net,10/11] af_unix: Fix wrong ioctl(SIOCATMARK) when consumed OOB skb is at the head.
    https://git.kernel.org/netdev/net/c/e400cfa38bb0
  - [v1,net,11/11] selftest: af_unix: Check SIOCATMARK after every send()/recv() in msg_oob.c.
    https://git.kernel.org/netdev/net/c/91b7186c8d14

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html