2015-10-02 0:05 GMT+03:00 Andrey Vagin <ava...@gmail.com>:
> From: Andrey Vagin <ava...@openvz.org>
>
> Now send with MSG_PEEK can return data from multiple SKBs.
>
> Unfortunately we take into account the peek offset for each skb,
> that is wrong. We need to apply the peek offset only once.
>
> In addition, the peek offset should be used only if MSG_PEEK is set.

The attached program can be used to reproduce the bug. It cycles in an
infinite loop without this patch, because recv() always returns data.

658   socketpair(PF_LOCAL, SOCK_STREAM, 0, [3, 4]) = 0
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   sendto(3, "Hello, World!", 13, MSG_DONTWAIT, NULL, 0) = 13
658   setsockopt(4, SOL_SOCKET, 0x2a /* SO_??? */, [0], 4) = 0
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   fstat(1, {st_mode=S_IFCHR|0666, st_rdev=makedev(1, 3), ...}) = 0
658   ioctl(1, TCGETS, 0x7fff54e57b30)  = -1 ENOTTY (Inappropriate
ioctl for device)
658   mmap(NULL, 4096, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f3a86827000
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
658   recvfrom(4, "Hello, World!H", 14, MSG_PEEK|MSG_DONTWAIT, NULL, NULL) = 14
...

>
> Cc: "David S. Miller" <da...@davemloft.net> (maintainer:NETWORKING
> Cc: Eric Dumazet <eduma...@google.com> (commit_signer:1/14=7%)
> Cc: Aaron Conole <acon...@bytheb.org>
> Fixes: 9f389e35674f ("af_unix: return data from multiple SKBs on recv() with 
> MSG_PEEK flag")
> Signed-off-by: Andrey Vagin <ava...@openvz.org>
> ---
>  net/unix/af_unix.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index ef31b40..94f6582 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2064,6 +2064,11 @@ static int unix_stream_read_generic(struct 
> unix_stream_read_state *state)
>                 goto out;
>         }
>
> +       if (flags & MSG_PEEK)
> +               skip = sk_peek_offset(sk, flags);
> +       else
> +               skip = 0;
> +
>         do {
>                 int chunk;
>                 struct sk_buff *skb, *last;
> @@ -2112,7 +2117,6 @@ unlock:
>                         break;
>                 }
>
> -               skip = sk_peek_offset(sk, flags);
>                 while (skip >= unix_skb_len(skb)) {
>                         skip -= unix_skb_len(skb);
>                         last = skb;
> @@ -2179,14 +2183,12 @@ unlock:
>                         if (UNIXCB(skb).fp)
>                                 scm.fp = scm_fp_dup(UNIXCB(skb).fp);
>
> -                       if (skip) {
> -                               sk_peek_offset_fwd(sk, chunk);
> -                               skip -= chunk;
> -                       }
> +                       sk_peek_offset_fwd(sk, chunk);
>
>                         if (UNIXCB(skb).fp)
>                                 break;
>
> +                       skip = 0;
>                         last = skb;
>                         last_len = skb->len;
>                         unix_state_lock(sk);
> --
> 2.4.3
>
#include <stdlib.h>
#include <stdio.h>
#include <errno.h>

#include <sys/types.h>
#include <sys/socket.h>

#define TEST_STRING "Hello, World!"

int main()
{
	char buf[15];
	int sks[2];
	int i, sk, ret;

	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sks))
		return 1;

	sk = sks[0];

	for (i = 0; i < 10; i++) {
		if (send(sk, TEST_STRING, sizeof(TEST_STRING) - 1, MSG_DONTWAIT) < 0)
			return 1;
	}

	sk = sks[1];

	ret = 0;
	if (setsockopt(sk, SOL_SOCKET, SO_PEEK_OFF, &ret, sizeof(int)))
		return 1;

	while (1) {
		int ret;
		ret = recv(sk, buf, sizeof(buf) - 1, MSG_DONTWAIT | MSG_PEEK);
		if (ret < 0) {
			if (errno == EAGAIN)
				break;
			return 1;
		}
		if (ret == 0)
			break;
		buf[ret] = 0;
		printf("msg: %s\n", buf);
	}

	return 0;
}

Reply via email to