Messages in this thread | | | Subject | Re: [PATCH net-next v4] skb_expand_head() adjust skb->truesize incorrectly | From | Vasily Averin <> | Date | Thu, 2 Sep 2021 10:13:29 +0300 |
| |
On 9/2/21 7:48 AM, Eric Dumazet wrote: > On 9/1/21 9:32 PM, Eric Dumazet wrote: >> I think you missed netem case, in particular >> skb_orphan_partial() which I already pointed out. >> >> You can setup a stack of virtual devices (tunnels), >> with a qdisc on them, before ip6_xmit() is finally called... >> >> Socket might have been closed already. >> >> To test your patch, you could force a skb_orphan_partial() at the beginning >> of skb_expand_head() (extending code coverage) > > To clarify : > > It is ok to 'downgrade' an skb->destructor having a ref on sk->sk_wmem_alloc to > something owning a ref on sk->refcnt. > > But the opposite operation (ref on sk->sk_refcnt --> ref on sk->sk_wmem_alloc) is not safe.
Could you please explain in more details, since I stil have a completely opposite point of view?
Every sk referenced in skb have sk_wmem_alloc > 9 It is assigned to 1 in sk_alloc and decremented right before last __sk_free(), inside both sk_free() sock_wfree() and __sock_wfree()
So it is safe to adjust skb->sk->sk_wmem_alloc, because alive skb keeps reference to alive sk and last one keeps sk_wmem_alloc > 0
So any destructor used sk->sk_refcnt will already have sk_wmem_alloc > 0, because last sock_put() calls sk_free().
However now I'm not sure in reversed direction. skb_set_owner_w() check !sk_fullsock(sk) and call sock_hold(sk); If sk->sk_refcnt can be 0 here (i.e. after execution of old destructor inside skb_orphan) -- it can be trigger pointed problem: "refcount_add() will trigger a warning (panic under KASAN)".
Could you please explain where I'm wrong?
Thank you, Vasily Averin
| |