netdev - Re: [PATCH 09/14] ipv6: hand dst refs to cork setup

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 11 Jan 2022 15:57:34 +0000
From:   Pavel Begunkov <asml.silence@...il.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 09/14] ipv6: hand dst refs to cork setup

On 1/11/22 15:39, Willem de Bruijn wrote:
> On Mon, Jan 10, 2022 at 8:25 PM Pavel Begunkov <asml.silence@...il.com> wrote:
>>
>> During cork->dst setup, ip6_make_skb() gets an additional reference to
>> a passed in dst. However, udpv6_sendmsg() doesn't need dst after calling
>> ip6_make_skb(), and so we can save two additional atomics by passing
>> dst references to ip6_make_skb(). udpv6_sendmsg() is the only caller, so
>> it's enough to make sure it doesn't use dst afterwards.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@...il.com>
>> ---
> 
> There are two patches 9/14

Weird, thanks for letting know

> 
>>   net/ipv6/ip6_output.c | 9 ++++++---
>>   net/ipv6/udp.c        | 3 ++-
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index 0cc490f2cfbf..6a7bba4dd04d 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -1356,6 +1356,8 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
>>          unsigned int mtu;
>>          struct ipv6_txoptions *nopt, *opt = ipc6->opt;
>>
>> +       cork->base.dst = &rt->dst;
>> +
> 
> Is there a reason to move this up from its original location next to
> the other cork initialization assignments?

ip6_setup_cork() consumes a dst ref now even in error cases, moved
it to not patch up all error returns in there. On the other hand
I can add dst_release() to callers when it failed.


> That the reference is taken in ip6_append_data for corked requests
> (once, in setup cork branch) and inherited from udpv6_send_skb
> otherwise is non-trivial. Worth a comment.

Will update in v2, thanks

-- 
Pavel Begunkov