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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Sat, 15 Aug 2020 02:09:06 +0000
From:   linmiaohe <linmiaohe@...wei.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Florian Westphal <fw@...len.de>,
        "martin.varghese@...ia.com" <martin.varghese@...ia.com>,
        "pshelar@....org" <pshelar@....org>,
        "dcaratti@...hat.com" <dcaratti@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        "Paolo Abeni" <pabeni@...hat.com>,
        Shmulik Ladkani <shmulik@...anetworks.com>,
        "Yadu Kishore" <kyk.segfault@...il.com>,
        "sowmini.varadhan@...cle.com" <sowmini.varadhan@...cle.com>,
        Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: add missing skb_uarg refcount increment in
 pskb_carve_inside_header()

Willem de Bruijn <willemdebruijn.kernel@...il.com> wrote:
>On Fri, Aug 14, 2020 at 9:20 AM linmiaohe <linmiaohe@...wei.com> wrote:
>>
>> Willem de Bruijn <willemdebruijn.kernel@...il.com> wrote:
>> >On Thu, Aug 13, 2020 at 2:16 PM Miaohe Lin <linmiaohe@...wei.com> wrote:
>> >>
>> >> If the skb is zcopied, we should increase the skb_uarg refcount 
>> >> before we involve skb_release_data(). See pskb_expand_head() as a reference.
>> >
>> >Did you manage to observe a bug through this datapath in practice?
>> >
>> >pskb_carve_inside_header is called
>> >  from pskb_carve
>> >    from pskb_extract
>> >      from rds_tcp_data_recv
>> >
>> >That receive path should not see any packets with zerocopy state associated.
>> >
>>
>> This works fine yet as its caller is limited. But we should take care of the skb_uarg refcount for future use.
>
>If a new application of this interface is proposed, the author will have to make sure that it is exercised correctly.

Sure. Let the author make sure that it is exercised correctly if a new application of this interface is proposed.

>> On the other hand, because this codepath should not see any packets 
>> with zerocopy state associated, then we should not call skb_orphan_frags here.

>I'm also not convinced that the skb_orphan_frags here are needed, given the only path is from tcp_read_sock.

Maybe just keep it here as it doesn't hurt even if it's really not needed.

Many thanks.

Powered by blists - more mailing lists