netdev - Re: [Patch net-next] call sk_dst_reset when set SO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAG3TDc0yAK8PvaYcZVgSD_wHEj2psqm-q8bkLnSzvFqfRVdPcA@mail.gmail.com>
Date:   Wed, 5 Dec 2018 16:34:22 -0800
From:   peng yu <yupeng0921@...il.com>
To:     eric.dumazet@...il.com
Cc:     Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [Patch net-next] call sk_dst_reset when set SO_DONTROUTE

In fack, my customer's issue is that he set SO_DONTROUTE by mistake.
He shouldn't do that. But after he set this flag, the connection has
no problem at first. After the sk_dst_cache expired for some reasons,
the connection stucked. I think the correct behavior is that the
connection should stuck immediately after set SO_DONTROUTE to 1.
On Wed, Dec 5, 2018 at 4:20 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>
>
>
> On 12/05/2018 04:13 PM, peng yu wrote:
> > The SO_DONTROUTE doesn't impact the TCP receiving path, but it should
> > block the ACK of the receiving packet. When there are too many packets
> > which are not ACKed, the client will stop to send packets, so the
> > sock.recv on the server side won't receive data after it received some
> > data. I extracted the test code from my customer's production
> > environment. The test code could reproduce the issue but it is not a
> > good example. I will rewrite a test code and re-submit the patch.
>
> Now I fully understand ;)
>
> Basically your customer is using SO_DONTROUTE to 'pause' incoming TCP traffic
> by not sending ACK.
>
> Interesting trick but quite hacky. I guess that sending ACK with 0 window
> would be less intrusive.
>
> > Wed, Dec 5, 2018 at 3:17 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >>
> >> On Wed, Dec 5, 2018 at 3:07 PM yupeng <yupeng0921@...il.com> wrote:
> >>>
> >>> after set SO_DONTROUTE to 1, the IP layer should not route packets if
> >>> the dest IP address is not in link scope. But if the socket has cached
> >>> the dst_entry, such packets would be routed until the sk_dst_cache
> >>> expires. So we should clean the sk_dst_cache when a user set
> >>> SO_DONTROUTE option. Below are server/client python scripts which
> >>> could reprodue this issue:
> >>>
> >>> server side code:
> >>> ==========================================================================
> >>> import socket
> >>> import struct
> >>>
> >>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> >>> s.bind(('0.0.0.0', 9000))
> >>> s.listen(1)
> >>> sock, addr = s.accept()
> >>> sock.setsockopt(socket.SOL_SOCKET, socket.SO_DONTROUTE, struct.pack('i', 1))
> >>> while True:
> >>>     data = sock.recv(1024) # here the sock.recv should not return anything
> >>
> >> Why is that so ?
> >>
> >> What is the relation of input path with the SO_DONTROUTE which is for TX ?
> >>
> >> sk_dst_reset(sk) should not impact receive side ?
> >>
> >> Thanks for providing a test !
> >>
> >>>     print(data)
> >>> ==========================================================================
> >>>
> >>> client side code:
> >>> ==========================================================================
> >>> import socket
> >>> import time
> >>>
> >>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> >>> s.connect(('server_address', 9000))
> >>> while True:
> >>>     s.send(b'foo')
> >>>     print('send foo')
> >>>     time.sleep(1)
> >>> ==========================================================================
> >>>
> >>> Signed-off-by: yupeng <yupeng0921@...il.com>
> >>> ---
> >>>  net/core/sock.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/net/core/sock.c b/net/core/sock.c
> >>> index f5bb89785e47..f00902c532cc 100644
> >>> --- a/net/core/sock.c
> >>> +++ b/net/core/sock.c
> >>> @@ -700,6 +700,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
> >>>                 break;
> >>>         case SO_DONTROUTE:
> >>>                 sock_valbool_flag(sk, SOCK_LOCALROUTE, valbool);
> >>> +               sk_dst_reset(sk);
> >>>                 break;
> >>>         case SO_BROADCAST:
> >>>                 sock_valbool_flag(sk, SOCK_BROADCAST, valbool);
> >>> --
> >>> 2.17.1
> >>>