Re: [PATCH] net: add missing skb_uarg refcount increment in pskb_carve_inside_header()

[linmiaohe]

Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote:
>On Fri, Aug 14, 2020 at 9:20 AM linmiaohe <linmiaohe@xxxxxxxxxx> wrote:
>>
>> Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote:
>> >On Thu, Aug 13, 2020 at 2:16 PM Miaohe Lin <linmiaohe@xxxxxxxxxx> 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.