[PATCH v1 net 0/2] af_unix: Fix SO_PEEK_OFF bug in unix_stream_read_generic().

[PATCH v1 net 0/2] af_unix: Fix SO_PEEK_OFF bug in unix_stream_read_generic().

[Kuniyuki Iwashima]

Miao Wang reported a bug of SO_PEEK_OFF on AF_UNIX SOCK_STREAM socket.

Patch 1 fixes the bug and Patch 2 adds a new selftest to cover the case.


Kuniyuki Iwashima (2):
  af_unix: Read sk_peek_offset() again after sleeping in
    unix_stream_read_generic().
  selftest: af_unix: Add test for SO_PEEK_OFF.

 net/unix/af_unix.c                            |   3 +-
 tools/testing/selftests/net/.gitignore        |   1 +
 tools/testing/selftests/net/af_unix/Makefile  |   1 +
 .../selftests/net/af_unix/so_peek_off.c       | 162 ++++++++++++++++++
 4 files changed, 165 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/af_unix/so_peek_off.c

-- 
2.52.0.rc1.455.g30608eb744-goog

[PATCH v1 net 1/2] af_unix: Read sk_peek_offset() again after sleeping in unix_stream_read_generic().

[Kuniyuki Iwashima]

Miao Wang reported a bug of SO_PEEK_OFF on AF_UNIX SOCK_STREAM
socket.

The unexpected behaviour is triggered when the peek offset is
larger than the recv queue and the thread is unblocked by new
data.

Let's assume a socket which has "aaaa" in the recv queue and
the peek offset is 4.

First, unix_stream_read_generic() reads the offset 4 and skips
the skb(s) of "aaaa" with the code below:

	skip = max(sk_peek_offset(sk, flags), 0);	/* @skip is 4. */

	do {
	...
		while (skip >= unix_skb_len(skb)) {
			skip -= unix_skb_len(skb);
		...
			skb = skb_peek_next(skb, &sk->sk_receive_queue);
			if (!skb)
				goto again;		/* @skip is 0. */
		}

The thread jumps to the 'again' label and goes to sleep since
new data has not arrived yet.

Later, new data "bbbb" unblocks the thread, and the thread jumps
to the 'redo:' label to restart the entire process from the first
skb in the recv queue.

	do {
		...
redo:
		...
		last = skb = skb_peek(&sk->sk_receive_queue);
		...
again:
		if (skb == NULL) {
			...
			timeo = unix_stream_data_wait(sk, timeo, last,
						      last_len, freezable);
			...
			goto redo;			/* @skip is 0 !! */

However, the peek offset is not reset in the path.

If the buffer size is 8, recv() will return "aaaabbbb" without
skipping any data, and the final offset will be 12 (the original
offset 4 + peeked skbs' length 8).

After sleeping in unix_stream_read_generic(), we have to fetch the
peek offset again.

Let's move the redo label before mutex_lock(&u->iolock).

Fixes: 9f389e35674f ("af_unix: return data from multiple SKBs on recv() with MSG_PEEK flag")
Reported-by: Miao Wang <shankerwangmiao@gmail.com>
Closes: https://lore.kernel.org/netdev/3B969F90-F51F-4B9D-AB1A-994D9A54D460@gmail.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/unix/af_unix.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 4124f472a02b..fa21519fce43 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2960,6 +2960,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 
 	u = unix_sk(sk);
 
+redo:
 	/* Lock the socket to prevent queue disordering
 	 * while sleeps in memcpy_tomsg
 	 */
@@ -2971,7 +2972,6 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 		struct sk_buff *skb, *last;
 		int chunk;
 
-redo:
 		unix_state_lock(sk);
 		if (sock_flag(sk, SOCK_DEAD)) {
 			err = -ECONNRESET;
@@ -3021,7 +3021,6 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 				goto out;
 			}
 
-			mutex_lock(&u->iolock);
 			goto redo;
 unlock:
 			unix_state_unlock(sk);
-- 
2.52.0.rc1.455.g30608eb744-goog

[PATCH v1 net 2/2] selftest: af_unix: Add test for SO_PEEK_OFF.

[Kuniyuki Iwashima]

The test covers various cases to verify SO_PEEK_OFF behaviour
for all AF_UNIX socket types.

two_chunks_blocking and two_chunks_overlap_blocking reproduce
the issue mentioned in the previous patch.

Without the patch, the two tests fail:

  #  RUN           so_peek_off.stream.two_chunks_blocking ...
  # so_peek_off.c:121:two_chunks_blocking:Expected 'bbbb' == 'aaaabbbb'.
  # two_chunks_blocking: Test terminated by assertion
  #          FAIL  so_peek_off.stream.two_chunks_blocking
  not ok 3 so_peek_off.stream.two_chunks_blocking

  #  RUN           so_peek_off.stream.two_chunks_overlap_blocking ...
  # so_peek_off.c:159:two_chunks_overlap_blocking:Expected 'bbbb' == 'aaaabbbb'.
  # two_chunks_overlap_blocking: Test terminated by assertion
  #          FAIL  so_peek_off.stream.two_chunks_overlap_blocking
  not ok 5 so_peek_off.stream.two_chunks_overlap_blocking

With the patch, all tests pass:

  # PASSED: 15 / 15 tests passed.
  # Totals: pass:15 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 tools/testing/selftests/net/.gitignore        |   1 +
 tools/testing/selftests/net/af_unix/Makefile  |   1 +
 .../selftests/net/af_unix/so_peek_off.c       | 162 ++++++++++++++++++
 3 files changed, 164 insertions(+)
 create mode 100644 tools/testing/selftests/net/af_unix/so_peek_off.c

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 439101b518ee..8f9850a71f54 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -45,6 +45,7 @@ skf_net_off
 socket
 so_incoming_cpu
 so_netns_cookie
+so_peek_off
 so_txtime
 so_rcv_listener
 stress_reuseport_listen
diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
index de805cbbdf69..528d14c598bb 100644
--- a/tools/testing/selftests/net/af_unix/Makefile
+++ b/tools/testing/selftests/net/af_unix/Makefile
@@ -6,6 +6,7 @@ TEST_GEN_PROGS := \
 	scm_inq \
 	scm_pidfd \
 	scm_rights \
+	so_peek_off \
 	unix_connect \
 # end of TEST_GEN_PROGS
 
diff --git a/tools/testing/selftests/net/af_unix/so_peek_off.c b/tools/testing/selftests/net/af_unix/so_peek_off.c
new file mode 100644
index 000000000000..1a77728128e5
--- /dev/null
+++ b/tools/testing/selftests/net/af_unix/so_peek_off.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2025 Google LLC */
+
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <sys/socket.h>
+
+#include "../../kselftest_harness.h"
+
+FIXTURE(so_peek_off)
+{
+	int fd[2];	/* 0: sender, 1: receiver */
+};
+
+FIXTURE_VARIANT(so_peek_off)
+{
+	int type;
+};
+
+FIXTURE_VARIANT_ADD(so_peek_off, stream)
+{
+	.type = SOCK_STREAM,
+};
+
+FIXTURE_VARIANT_ADD(so_peek_off, dgram)
+{
+	.type = SOCK_DGRAM,
+};
+
+FIXTURE_VARIANT_ADD(so_peek_off, seqpacket)
+{
+	.type = SOCK_SEQPACKET,
+};
+
+FIXTURE_SETUP(so_peek_off)
+{
+	struct timeval timeout = {
+		.tv_sec = 0,
+		.tv_usec = 3000,
+	};
+	int ret;
+
+	ret = socketpair(AF_UNIX, variant->type, 0, self->fd);
+	ASSERT_EQ(0, ret);
+
+	ret = setsockopt(self->fd[1], SOL_SOCKET, SO_RCVTIMEO_NEW,
+			 &timeout, sizeof(timeout));
+	ASSERT_EQ(0, ret);
+
+	ret = setsockopt(self->fd[1], SOL_SOCKET, SO_PEEK_OFF,
+			 &(int){0}, sizeof(int));
+	ASSERT_EQ(0, ret);
+}
+
+FIXTURE_TEARDOWN(so_peek_off)
+{
+	close_range(self->fd[0], self->fd[1], 0);
+}
+
+#define sendeq(fd, str, flags)					\
+	do {							\
+		int bytes, len = strlen(str);			\
+								\
+		bytes = send(fd, str, len, flags);		\
+		ASSERT_EQ(len, bytes);				\
+	} while (0)
+
+#define recveq(fd, str, buflen, flags)				\
+	do {							\
+		char buf[(buflen) + 1] = {};			\
+		int bytes;					\
+								\
+		bytes = recv(fd, buf, buflen, flags);		\
+		ASSERT_NE(-1, bytes);				\
+		ASSERT_STREQ(str, buf);				\
+	} while (0)
+
+#define async							\
+	for (pid_t pid = (pid = fork(),				\
+			  pid < 0 ?				\
+			  __TH_LOG("Failed to start async {}"),	\
+			  _metadata->exit_code = KSFT_FAIL,	\
+			  __bail(1, _metadata),			\
+			  0xdead :				\
+			  pid);					\
+	     !pid; exit(0))
+
+TEST_F(so_peek_off, single_chunk)
+{
+	sendeq(self->fd[0], "aaaabbbb", 0);
+
+	recveq(self->fd[1], "aaaa", 4, MSG_PEEK);
+	recveq(self->fd[1], "bbbb", 100, MSG_PEEK);
+}
+
+TEST_F(so_peek_off, two_chunks)
+{
+	sendeq(self->fd[0], "aaaa", 0);
+	sendeq(self->fd[0], "bbbb", 0);
+
+	recveq(self->fd[1], "aaaa", 4, MSG_PEEK);
+	recveq(self->fd[1], "bbbb", 100, MSG_PEEK);
+}
+
+TEST_F(so_peek_off, two_chunks_blocking)
+{
+	async {
+		usleep(1000);
+		sendeq(self->fd[0], "aaaa", 0);
+	}
+
+	recveq(self->fd[1], "aaaa", 4, MSG_PEEK);
+
+	async {
+		usleep(1000);
+		sendeq(self->fd[0], "bbbb", 0);
+	}
+
+	/* goto again; -> goto redo; in unix_stream_read_generic(). */
+	recveq(self->fd[1], "bbbb", 100, MSG_PEEK);
+}
+
+TEST_F(so_peek_off, two_chunks_overlap)
+{
+	sendeq(self->fd[0], "aaaa", 0);
+	recveq(self->fd[1], "aa", 2, MSG_PEEK);
+
+	sendeq(self->fd[0], "bbbb", 0);
+
+	if (variant->type == SOCK_STREAM) {
+		/* SOCK_STREAM tries to fill the buffer. */
+		recveq(self->fd[1], "aabb", 4, MSG_PEEK);
+		recveq(self->fd[1], "bb", 100, MSG_PEEK);
+	} else {
+		/* SOCK_DGRAM and SOCK_SEQPACKET returns at the skb boundary. */
+		recveq(self->fd[1], "aa", 100, MSG_PEEK);
+		recveq(self->fd[1], "bbbb", 100, MSG_PEEK);
+	}
+}
+
+TEST_F(so_peek_off, two_chunks_overlap_blocking)
+{
+	async {
+		usleep(1000);
+		sendeq(self->fd[0], "aaaa", 0);
+	}
+
+	recveq(self->fd[1], "aa", 2, MSG_PEEK);
+
+	async {
+		usleep(1000);
+		sendeq(self->fd[0], "bbbb", 0);
+	}
+
+	/* Even SOCK_STREAM does not wait if at least one byte is read. */
+	recveq(self->fd[1], "aa", 100, MSG_PEEK);
+
+	recveq(self->fd[1], "bbbb", 100, MSG_PEEK);
+}
+
+TEST_HARNESS_MAIN
-- 
2.52.0.rc1.455.g30608eb744-goog

Re: [PATCH v1 net 2/2] selftest: af_unix: Add test for SO_PEEK_OFF.

[Miao Wang]

> 2025年11月18日 01:47，Kuniyuki Iwashima <kuniyu@google.com> 写道：
> 
> The test covers various cases to verify SO_PEEK_OFF behaviour
> for all AF_UNIX socket types.
> 
> two_chunks_blocking and two_chunks_overlap_blocking reproduce
> the issue mentioned in the previous patch.
> 
> Without the patch, the two tests fail:
> 
>  #  RUN           so_peek_off.stream.two_chunks_blocking ...
>  # so_peek_off.c:121:two_chunks_blocking:Expected 'bbbb' == 'aaaabbbb'.
>  # two_chunks_blocking: Test terminated by assertion
>  #          FAIL  so_peek_off.stream.two_chunks_blocking
>  not ok 3 so_peek_off.stream.two_chunks_blocking
> 
>  #  RUN           so_peek_off.stream.two_chunks_overlap_blocking ...
>  # so_peek_off.c:159:two_chunks_overlap_blocking:Expected 'bbbb' == 'aaaabbbb'.
>  # two_chunks_overlap_blocking: Test terminated by assertion
>  #          FAIL  so_peek_off.stream.two_chunks_overlap_blocking
>  not ok 5 so_peek_off.stream.two_chunks_overlap_blocking
> 
> With the patch, all tests pass:
> 
>  # PASSED: 15 / 15 tests passed.
>  # Totals: pass:15 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
> tools/testing/selftests/net/.gitignore        |   1 +
> tools/testing/selftests/net/af_unix/Makefile  |   1 +
> .../selftests/net/af_unix/so_peek_off.c       | 162 ++++++++++++++++++
> 3 files changed, 164 insertions(+)
> create mode 100644 tools/testing/selftests/net/af_unix/so_peek_off.c
> 

Hi,

Many thanks for your patch. I wonder if at the end of each
test, a normal recv() without MGS_PEEK can be called to check
if it can receive all the content in the receiving buffer and
check if SO_PEEK_OFF becomes back to zero.

Cheers,

Miao Wang

Re: [PATCH v1 net 2/2] selftest: af_unix: Add test for SO_PEEK_OFF.

[Kuniyuki Iwashima]

On Mon, Nov 17, 2025 at 3:10 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>
>
> > 2025年11月18日 01:47，Kuniyuki Iwashima <kuniyu@google.com> 写道：
> >
> > The test covers various cases to verify SO_PEEK_OFF behaviour
> > for all AF_UNIX socket types.
> >
> > two_chunks_blocking and two_chunks_overlap_blocking reproduce
> > the issue mentioned in the previous patch.
> >
> > Without the patch, the two tests fail:
> >
> >  #  RUN           so_peek_off.stream.two_chunks_blocking ...
> >  # so_peek_off.c:121:two_chunks_blocking:Expected 'bbbb' == 'aaaabbbb'.
> >  # two_chunks_blocking: Test terminated by assertion
> >  #          FAIL  so_peek_off.stream.two_chunks_blocking
> >  not ok 3 so_peek_off.stream.two_chunks_blocking
> >
> >  #  RUN           so_peek_off.stream.two_chunks_overlap_blocking ...
> >  # so_peek_off.c:159:two_chunks_overlap_blocking:Expected 'bbbb' == 'aaaabbbb'.
> >  # two_chunks_overlap_blocking: Test terminated by assertion
> >  #          FAIL  so_peek_off.stream.two_chunks_overlap_blocking
> >  not ok 5 so_peek_off.stream.two_chunks_overlap_blocking
> >
> > With the patch, all tests pass:
> >
> >  # PASSED: 15 / 15 tests passed.
> >  # Totals: pass:15 fail:0 xfail:0 xpass:0 skip:0 error:0
> >
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> > tools/testing/selftests/net/.gitignore        |   1 +
> > tools/testing/selftests/net/af_unix/Makefile  |   1 +
> > .../selftests/net/af_unix/so_peek_off.c       | 162 ++++++++++++++++++
> > 3 files changed, 164 insertions(+)
> > create mode 100644 tools/testing/selftests/net/af_unix/so_peek_off.c
> >
>
> Hi,
>
> Many thanks for your patch. I wonder if at the end of each
> test, a normal recv() without MGS_PEEK can be called to check
> if it can receive all the content in the receiving buffer and
> check if SO_PEEK_OFF becomes back to zero.

I'd keep this for now as it covers the fix in the previous patch.
We could add more tests in net-next if we want.

Thanks!

Re: [PATCH v1 net 2/2] selftest: af_unix: Add test for SO_PEEK_OFF.

[Jakub Kicinski]

On Mon, 17 Nov 2025 17:47:11 +0000 Kuniyuki Iwashima wrote:
> diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
> index 439101b518ee..8f9850a71f54 100644
> --- a/tools/testing/selftests/net/.gitignore
> +++ b/tools/testing/selftests/net/.gitignore
> @@ -45,6 +45,7 @@ skf_net_off
>  socket
>  so_incoming_cpu
>  so_netns_cookie
> +so_peek_off

NIPA is complaining that we're missing the binary name in gitignore.
Probably not worth respinning for this but in the future let's start
using af_unix/.gitignore rather than the parent's .gitignore?

>  so_txtime
>  so_rcv_listener
>  stress_reuseport_listen
> diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
> index de805cbbdf69..528d14c598bb 100644
> --- a/tools/testing/selftests/net/af_unix/Makefile
> +++ b/tools/testing/selftests/net/af_unix/Makefile
> @@ -6,6 +6,7 @@ TEST_GEN_PROGS := \
>  	scm_inq \
>  	scm_pidfd \
>  	scm_rights \
> +	so_peek_off \
>  	unix_connect \
>  # end of TEST_GEN_PROGS

Re: [PATCH v1 net 2/2] selftest: af_unix: Add test for SO_PEEK_OFF.

[Kuniyuki Iwashima]

On Mon, Nov 17, 2025 at 5:26 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 17 Nov 2025 17:47:11 +0000 Kuniyuki Iwashima wrote:
> > diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
> > index 439101b518ee..8f9850a71f54 100644
> > --- a/tools/testing/selftests/net/.gitignore
> > +++ b/tools/testing/selftests/net/.gitignore
> > @@ -45,6 +45,7 @@ skf_net_off
> >  socket
> >  so_incoming_cpu
> >  so_netns_cookie
> > +so_peek_off
>
> NIPA is complaining that we're missing the binary name in gitignore.
> Probably not worth respinning for this but in the future let's start
> using af_unix/.gitignore rather than the parent's .gitignore?

Sure, I'll move all of them under af_unix in net-next, including
this one.

https://patchwork.kernel.org/project/netdevbpf/patch/20251113112802.44657-1-adelodunolaoluwa@yahoo.com/

Thanks!

Re: [PATCH v1 net 0/2] af_unix: Fix SO_PEEK_OFF bug in unix_stream_read_generic().

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 17 Nov 2025 17:47:09 +0000 you wrote:
> Miao Wang reported a bug of SO_PEEK_OFF on AF_UNIX SOCK_STREAM socket.
> 
> Patch 1 fixes the bug and Patch 2 adds a new selftest to cover the case.
> 
> 
> Kuniyuki Iwashima (2):
>   af_unix: Read sk_peek_offset() again after sleeping in
>     unix_stream_read_generic().
>   selftest: af_unix: Add test for SO_PEEK_OFF.
> 
> [...]

Here is the summary with links:
  - [v1,net,1/2] af_unix: Read sk_peek_offset() again after sleeping in unix_stream_read_generic().
    https://git.kernel.org/netdev/net/c/7bf3a476ce43
  - [v1,net,2/2] selftest: af_unix: Add test for SO_PEEK_OFF.
    https://git.kernel.org/netdev/net/c/e1bb28bf13f4

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