netdev - Re: [PATCH net v4 0/3] ipv6: udp6: set dst cache for a connected sk if current not valid

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]

Date:   Fri, 30 Mar 2018 14:19:14 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Alexey Kodanev <alexey.kodanev@...cle.com>
CC:     <netdev@...r.kernel.org>, Eric Dumazet <edumazet@...gle.com>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH net v4 0/3] ipv6: udp6: set dst cache for a connected sk
 if current not valid

On Fri, Mar 30, 2018 at 08:53:06PM +0300, Alexey Kodanev wrote:
> A new RTF_CACHE route can be created with the socket's dst cache
> update between the below calls in udpv6_sendmsg(), when datagram
> sending results to ICMPV6_PKT_TOOBIG error:
> 
>    dst = ip6_sk_dst_lookup_flow(...)
>    ...
> release_dst:
>     if (dst) {
>         if (connected) {
>             ip6_dst_store(sk, dst)
> 
> Therefore, the new socket's dst cache reset to the old one on
> "release_dst:".
> 
> The first two patches prepare the code to store dst cache
> with ip6_sk_dst_lookup_flow():
> 
>   * the first patch adds ip6_dst_store_flow() function with
>     commonly used source and destiantion addresses checks using
>     the flow information.
> 
>   * the second patch adds new argument to ip6_sk_dst_lookup_flow()
>     and ability to store dst in the socket's cache. Also, the two
>     users of the function are updated without enabling the new
>     behavior: pingv6_sendmsg() and udpv6_sendmsg().
> 
> The last patch contains the actual fix that removes sk dst cache
> update in the end of udpv6_sendmsg(), and allows to do it in
> ip6_sk_dst_lookup_flow().
> 
> v4: * fix the error in the build of ip_dst_store_flow() reported by
>       kbuild test robot due to missing checks for CONFIG_IPV6: add
>       new function to ip6_output.c instead of ip6_route.h
Thanks for the patches!

Instead of ip6_output.c, would net/ipv6/route.c be a better
place for the new ip6_sk_dst_store_flow()?

Others LGTM.  

>     * add 'const' to struct flowi6 in ip6_dst_store_flow()
>     * minor commit messages fixes
> 
> v3: * instead of moving ip6_dst_store() above udp_v6_send_skb(),
>       update socket's dst cache inside ip6_sk_dst_lookup_flow()
>       if the current one is invalid
>     * the issue not reproduced in 4.1, but starting from 4.2. Add
>       one more 'Fixes:' commit that creates new RTF_CACHE route.
>       Though, it is also mentioned in the first one
> 
> 
> Alexey Kodanev (3):
>   ipv6: add a wrapper for ip6_dst_store() with flowi6 checks
>   ipv6: allow to cache dst for a connected sk in ip6_sk_dst_lookup_flow()
>   ipv6: udp6: set dst cache for a connected sk if current not valid
> 
>  include/net/ipv6.h    |  6 ++++--
>  net/ipv6/datagram.c   |  9 +--------
>  net/ipv6/ip6_output.c | 32 +++++++++++++++++++++++++++++---
>  net/ipv6/ping.c       |  2 +-
>  net/ipv6/udp.c        | 21 ++-------------------
>  5 files changed, 37 insertions(+), 33 deletions(-)
> 
> -- 
> 1.8.3.1
>