netdev - Re: [PATCH] net: add missing skb_uarg refcount increment in pskb_carve_inside

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>] [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.