So, what is the result of discussion here?

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 12/23/2016 10:19 AM, Cyrill Gorcunov wrote:
On Fri, Dec 23, 2016 at 09:59:28AM +0300, Cyrill Gorcunov wrote:
On Thu, Dec 22, 2016 at 04:45:10PM -0800, Andrey Vagin wrote:

Actually, this patch breaks the old behaviour even when MSG_PEEK isn't set.

I was thinking a bit more and I don't understand why it is a problem. If
we can't fill a buffer, an error will be returned and a user will be able
to set peek_offset to get these data again.

A user doesn't have to set peek again, without the patch the internal state
of sk is context-dependant, which is broken design. Take a look on unix
sockets code, they DON'T modify offset if something earlier failed for
exactly same reason.

Another option -- set offset only iff data copying passed.
---
From: Cyrill Gorcunov <gorcu...@openvz.org>
Subject: [PATCH rh7 v2] netlink: Don't manipulate @sk_peek_off if data fetching 
failed

When skb_copy_datagram_iovec called to fetch queued data
it may fail with EFAULT and if MSG_PEEK set by a caller
the position get advanced even if data hasn't been read.
So we might loose data bits here on subsequent recvmsg
calls. Instead lets exit early with error.

In sake of https://jira.sw.ru/browse/PSBM-57921

CC: Andrey Vagin <ava...@openvz.org>
Signed-off-by: Cyrill Gorcunov <gorcu...@openvz.org>
---
 net/netlink/af_netlink.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Index: linux-pcs7.git/net/netlink/af_netlink.c
===================================================================
--- linux-pcs7.git.orig/net/netlink/af_netlink.c
+++ linux-pcs7.git/net/netlink/af_netlink.c
@@ -2473,11 +2473,12 @@ static int netlink_recvmsg(struct kiocb

        skb_reset_transport_header(data_skb);
        err = skb_copy_datagram_iovec(data_skb, skip, msg->msg_iov, copied);
-
-       if (flags & MSG_PEEK)
-               sk_peek_offset_fwd(sk, copied);
-       else
-               sk_peek_offset_bwd(sk, skb->len);
+       if (!err) {
+               if (flags & MSG_PEEK)
+                       sk_peek_offset_fwd(sk, copied);
+               else
+                       sk_peek_offset_bwd(sk, skb->len);
+       }

        if (msg->msg_name) {
                struct sockaddr_nl *addr = (struct sockaddr_nl *)msg->msg_name;
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to