* [PATCH net v8] skb_expand_head() adjust skb->truesize incorrectly [not found] <20210920111259.18f9cc01@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> @ 2021-09-20 21:41 ` Vasily Averin 2021-09-21 5:21 ` Vasily Averin 0 siblings, 1 reply; 11+ messages in thread From: Vasily Averin @ 2021-09-20 21:41 UTC (permalink / raw) To: Christoph Paasch, Jakub Kicinski, Eric Dumazet Cc: David S. Miller, Hideaki YOSHIFUJI, David Ahern, netdev, linux-kernel, kernel, Julian Wiedmann Christoph Paasch reports [1] about incorrect skb->truesize after skb_expand_head() call in ip6_xmit. This may happen because of two reasons: - skb_set_owner_w() for newly cloned skb is called too early, before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. - pskb_expand_head() does not adjust truesize in (skb->sk) case. In this case sk->sk_wmem_alloc should be adjusted too. [1] https://lkml.org/lkml/2021/8/20/1082 Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") Reported-by: Christoph Paasch <christoph.paasch@gmail.com> Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- v8: clone non-wmem skb V7 (from kuba@): shift more magic into helpers, follow Eric's advice and don't inherit non-wmem skbs for now v6: fixed delta, improved comments v5: fixed else condition, thanks to Eric reworked update of expanded skb, added corresponding comments v4: decided to use is_skb_wmem() after pskb_expand_head() call fixed 'return (EXPRESSION);' in os_skb_wmem according to Eric Dumazet v3: removed __pskb_expand_head(), added is_skb_wmem() helper for skb with wmem-compatible destructors there are 2 ways to use it: - before pskb_expand_head(), to create skb clones - after successfull pskb_expand_head() to change owner on extended skb. v2: based on patch version from Eric Dumazet, added __pskb_expand_head() function, which can be forced to adjust skb->truesize and sk->sk_wmem_alloc. --- include/net/sock.h | 1 + net/core/skbuff.c | 33 +++++++++++++++++++++------------ net/core/sock.c | 8 ++++++++ 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/include/net/sock.h b/include/net/sock.h index 95b2577..173d58c 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1695,6 +1695,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force, gfp_t priority); void __sock_wfree(struct sk_buff *skb); void sock_wfree(struct sk_buff *skb); +bool is_skb_wmem(const struct sk_buff *skb); struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size, gfp_t priority); void skb_orphan_partial(struct sk_buff *skb); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index f931176..4b49f63 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1804,30 +1804,39 @@ struct sk_buff *skb_realloc_headroom(struct sk_buff *skb, unsigned int headroom) struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) { int delta = headroom - skb_headroom(skb); + int osize = skb_end_offset(skb); + struct sock *sk = skb->sk; if (WARN_ONCE(delta <= 0, "%s is expecting an increase in the headroom", __func__)) return skb; + delta = SKB_DATA_ALIGN(delta); /* pskb_expand_head() might crash, if skb is shared */ - if (skb_shared(skb)) { + if (skb_shared(skb) || !is_skb_wmem(skb)) { struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); - if (likely(nskb)) { - if (skb->sk) - skb_set_owner_w(nskb, skb->sk); - consume_skb(skb); - } else { - kfree_skb(skb); - } + if (unlikely(!nskb)) + goto fail; + + if (sk) + skb_set_owner_w(nskb, sk); + consume_skb(skb); skb = nskb; } - if (skb && - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { - kfree_skb(skb); - skb = NULL; + if (pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) + goto fail; + + if (sk) { + delta = skb_end_offset(skb) - osize; + refcount_add(delta, &sk->sk_wmem_alloc); + skb->truesize += delta; } return skb; + +fail: + kfree_skb(skb); + return NULL; } EXPORT_SYMBOL(skb_expand_head); diff --git a/net/core/sock.c b/net/core/sock.c index 950f1e7..6cbda43 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -2227,6 +2227,14 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) } EXPORT_SYMBOL(skb_set_owner_w); +bool is_skb_wmem(const struct sk_buff *skb) +{ + return skb->destructor == sock_wfree || + skb->destructor == __sock_wfree || + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); +} +EXPORT_SYMBOL(is_skb_wmem); + static bool can_skb_orphan_partial(const struct sk_buff *skb) { #ifdef CONFIG_TLS_DEVICE -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net v8] skb_expand_head() adjust skb->truesize incorrectly 2021-09-20 21:41 ` [PATCH net v8] skb_expand_head() adjust skb->truesize incorrectly Vasily Averin @ 2021-09-21 5:21 ` Vasily Averin 2021-10-04 13:00 ` [PATCH net v9] " Vasily Averin 0 siblings, 1 reply; 11+ messages in thread From: Vasily Averin @ 2021-09-21 5:21 UTC (permalink / raw) To: Christoph Paasch, Jakub Kicinski, Eric Dumazet Cc: David S. Miller, Hideaki YOSHIFUJI, David Ahern, netdev, linux-kernel, kernel, Julian Wiedmann On 9/21/21 12:41 AM, Vasily Averin wrote: > Christoph Paasch reports [1] about incorrect skb->truesize > after skb_expand_head() call in ip6_xmit. > This may happen because of two reasons: > - skb_set_owner_w() for newly cloned skb is called too early, > before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. > - pskb_expand_head() does not adjust truesize in (skb->sk) case. > In this case sk->sk_wmem_alloc should be adjusted too. > > [1] https://lkml.org/lkml/2021/8/20/1082 > > Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") > Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") > Reported-by: Christoph Paasch <christoph.paasch@gmail.com> > Signed-off-by: Vasily Averin <vvs@virtuozzo.com> > --- > v8: clone non-wmem skb > V7 (from kuba@): > shift more magic into helpers, > follow Eric's advice and don't inherit non-wmem skbs for now > v6: fixed delta, > improved comments > v5: fixed else condition, thanks to Eric > reworked update of expanded skb, > added corresponding comments > v4: decided to use is_skb_wmem() after pskb_expand_head() call > fixed 'return (EXPRESSION);' in os_skb_wmem according to Eric Dumazet > v3: removed __pskb_expand_head(), > added is_skb_wmem() helper for skb with wmem-compatible destructors > there are 2 ways to use it: > - before pskb_expand_head(), to create skb clones > - after successfull pskb_expand_head() to change owner on extended skb. > v2: based on patch version from Eric Dumazet, > added __pskb_expand_head() function, which can be forced > to adjust skb->truesize and sk->sk_wmem_alloc. > --- > include/net/sock.h | 1 + > net/core/skbuff.c | 33 +++++++++++++++++++++------------ > net/core/sock.c | 8 ++++++++ > 3 files changed, 30 insertions(+), 12 deletions(-) > > diff --git a/include/net/sock.h b/include/net/sock.h > index 95b2577..173d58c 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -1695,6 +1695,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force, > gfp_t priority); > void __sock_wfree(struct sk_buff *skb); > void sock_wfree(struct sk_buff *skb); > +bool is_skb_wmem(const struct sk_buff *skb); > struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size, > gfp_t priority); > void skb_orphan_partial(struct sk_buff *skb); > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index f931176..4b49f63 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -1804,30 +1804,39 @@ struct sk_buff *skb_realloc_headroom(struct sk_buff *skb, unsigned int headroom) > struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) > { > int delta = headroom - skb_headroom(skb); > + int osize = skb_end_offset(skb); > + struct sock *sk = skb->sk; > > if (WARN_ONCE(delta <= 0, > "%s is expecting an increase in the headroom", __func__)) > return skb; > > + delta = SKB_DATA_ALIGN(delta); > /* pskb_expand_head() might crash, if skb is shared */ > - if (skb_shared(skb)) { > + if (skb_shared(skb) || !is_skb_wmem(skb)) { > struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); > > - if (likely(nskb)) { > - if (skb->sk) > - skb_set_owner_w(nskb, skb->sk); > - consume_skb(skb); > - } else { > - kfree_skb(skb); > - } > + if (unlikely(!nskb)) > + goto fail; > + > + if (sk) > + skb_set_owner_w(nskb, sk); > + consume_skb(skb); > skb = nskb; > } > - if (skb && > - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { > - kfree_skb(skb); > - skb = NULL; > + if (pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) > + goto fail; > + > + if (sk) { sock_edemux check is still required here too. > + delta = skb_end_offset(skb) - osize; > + refcount_add(delta, &sk->sk_wmem_alloc); > + skb->truesize += delta; > } > return skb; > + > +fail: > + kfree_skb(skb); > + return NULL; > } > EXPORT_SYMBOL(skb_expand_head); > > diff --git a/net/core/sock.c b/net/core/sock.c > index 950f1e7..6cbda43 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -2227,6 +2227,14 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) > } > EXPORT_SYMBOL(skb_set_owner_w); > > +bool is_skb_wmem(const struct sk_buff *skb) > +{ > + return skb->destructor == sock_wfree || > + skb->destructor == __sock_wfree || > + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); > +} > +EXPORT_SYMBOL(is_skb_wmem); > + > static bool can_skb_orphan_partial(const struct sk_buff *skb) > { > #ifdef CONFIG_TLS_DEVICE > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net v9] skb_expand_head() adjust skb->truesize incorrectly 2021-09-21 5:21 ` Vasily Averin @ 2021-10-04 13:00 ` Vasily Averin 2021-10-04 13:14 ` Vasily Averin 2021-10-04 19:26 ` Eric Dumazet 0 siblings, 2 replies; 11+ messages in thread From: Vasily Averin @ 2021-10-04 13:00 UTC (permalink / raw) To: Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel Christoph Paasch reports [1] about incorrect skb->truesize after skb_expand_head() call in ip6_xmit. This may happen because of two reasons: - skb_set_owner_w() for newly cloned skb is called too early, before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. - pskb_expand_head() does not adjust truesize in (skb->sk) case. In this case sk->sk_wmem_alloc should be adjusted too. [1] https://lkml.org/lkml/2021/8/20/1082 Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") Reported-by: Christoph Paasch <christoph.paasch@gmail.com> Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- v9: restored sock_edemux check v8: clone non-wmem skb v7 (from kuba@): shift more magic into helpers, follow Eric's advice and don't inherit non-wmem skbs for now v6: fixed delta, improved comments v5: fixed else condition, thanks to Eric reworked update of expanded skb, added corresponding comments v4: decided to use is_skb_wmem() after pskb_expand_head() call fixed 'return (EXPRESSION);' in os_skb_wmem according to Eric Dumazet v3: removed __pskb_expand_head(), added is_skb_wmem() helper for skb with wmem-compatible destructors there are 2 ways to use it: - before pskb_expand_head(), to create skb clones - after successfull pskb_expand_head() to change owner on extended skb. v2: based on patch version from Eric Dumazet, added __pskb_expand_head() function, which can be forced to adjust skb->truesize and sk->sk_wmem_alloc. --- include/net/sock.h | 1 + net/core/skbuff.c | 35 ++++++++++++++++++++++------------- net/core/sock.c | 8 ++++++++ 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/include/net/sock.h b/include/net/sock.h index 66a9a90f9558..a547651d46a7 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1695,6 +1695,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force, gfp_t priority); void __sock_wfree(struct sk_buff *skb); void sock_wfree(struct sk_buff *skb); +bool is_skb_wmem(const struct sk_buff *skb); struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size, gfp_t priority); void skb_orphan_partial(struct sk_buff *skb); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 2170bea2c7de..8cb6d360cda5 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1804,30 +1804,39 @@ EXPORT_SYMBOL(skb_realloc_headroom); struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) { int delta = headroom - skb_headroom(skb); + int osize = skb_end_offset(skb); + struct sock *sk = skb->sk; if (WARN_ONCE(delta <= 0, "%s is expecting an increase in the headroom", __func__)) return skb; - /* pskb_expand_head() might crash, if skb is shared */ - if (skb_shared(skb)) { + delta = SKB_DATA_ALIGN(delta); + /* pskb_expand_head() might crash, if skb is shared. */ + if (skb_shared(skb) || !is_skb_wmem(skb)) { struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); - if (likely(nskb)) { - if (skb->sk) - skb_set_owner_w(nskb, skb->sk); - consume_skb(skb); - } else { - kfree_skb(skb); - } + if (unlikely(!nskb)) + goto fail; + + if (sk) + skb_set_owner_w(nskb, sk); + consume_skb(skb); skb = nskb; } - if (skb && - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { - kfree_skb(skb); - skb = NULL; + if (pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) + goto fail; + + if (sk && skb->destructor != sock_edemux) { + delta = skb_end_offset(skb) - osize; + refcount_add(delta, &sk->sk_wmem_alloc); + skb->truesize += delta; } return skb; + +fail: + kfree_skb(skb); + return NULL; } EXPORT_SYMBOL(skb_expand_head); diff --git a/net/core/sock.c b/net/core/sock.c index 62627e868e03..1932755ae9ba 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -2227,6 +2227,14 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) } EXPORT_SYMBOL(skb_set_owner_w); +bool is_skb_wmem(const struct sk_buff *skb) +{ + return skb->destructor == sock_wfree || + skb->destructor == __sock_wfree || + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); +} +EXPORT_SYMBOL(is_skb_wmem); + static bool can_skb_orphan_partial(const struct sk_buff *skb) { #ifdef CONFIG_TLS_DEVICE -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net v9] skb_expand_head() adjust skb->truesize incorrectly 2021-10-04 13:00 ` [PATCH net v9] " Vasily Averin @ 2021-10-04 13:14 ` Vasily Averin 2021-10-04 19:26 ` Eric Dumazet 1 sibling, 0 replies; 11+ messages in thread From: Vasily Averin @ 2021-10-04 13:14 UTC (permalink / raw) To: Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel Dear Eric, could you please take look at this patch? Original issue reported by Christoph Paasch is still not fixed, however now buggy paches was merged into upstream. v6 patch version [1] fixes the problem by careful change of destructor on existing skb. I still think it is correct however I'm agree it requires careful review. [1] https://lkml.org/lkml/2021/9/6/584 This patch version is more simple and returns to cloning of non-wmem skb. Both variants (i.e. this one and v6) resolves the problem. Could you please review the patches, select one of them or propose some better solution? Thank you, Vasily Averin On 10/4/21 4:00 PM, Vasily Averin wrote: > Christoph Paasch reports [1] about incorrect skb->truesize > after skb_expand_head() call in ip6_xmit. > This may happen because of two reasons: > - skb_set_owner_w() for newly cloned skb is called too early, > before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. > - pskb_expand_head() does not adjust truesize in (skb->sk) case. > In this case sk->sk_wmem_alloc should be adjusted too. > > [1] https://lkml.org/lkml/2021/8/20/1082 > > Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") > Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") > Reported-by: Christoph Paasch <christoph.paasch@gmail.com> > Signed-off-by: Vasily Averin <vvs@virtuozzo.com> > --- > v9: restored sock_edemux check > v8: clone non-wmem skb > v7 (from kuba@): > shift more magic into helpers, > follow Eric's advice and don't inherit non-wmem skbs for now > v6: fixed delta, > improved comments > v5: fixed else condition, thanks to Eric > reworked update of expanded skb, > added corresponding comments > v4: decided to use is_skb_wmem() after pskb_expand_head() call > fixed 'return (EXPRESSION);' in os_skb_wmem according to Eric Dumazet > v3: removed __pskb_expand_head(), > added is_skb_wmem() helper for skb with wmem-compatible destructors > there are 2 ways to use it: > - before pskb_expand_head(), to create skb clones > - after successfull pskb_expand_head() to change owner on extended > skb. > v2: based on patch version from Eric Dumazet, > added __pskb_expand_head() function, which can be forced > to adjust skb->truesize and sk->sk_wmem_alloc. > --- > include/net/sock.h | 1 + > net/core/skbuff.c | 35 ++++++++++++++++++++++------------- > net/core/sock.c | 8 ++++++++ > 3 files changed, 31 insertions(+), 13 deletions(-) > > diff --git a/include/net/sock.h b/include/net/sock.h > index 66a9a90f9558..a547651d46a7 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -1695,6 +1695,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force, > gfp_t priority); > void __sock_wfree(struct sk_buff *skb); > void sock_wfree(struct sk_buff *skb); > +bool is_skb_wmem(const struct sk_buff *skb); > struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size, > gfp_t priority); > void skb_orphan_partial(struct sk_buff *skb); > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 2170bea2c7de..8cb6d360cda5 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -1804,30 +1804,39 @@ EXPORT_SYMBOL(skb_realloc_headroom); > struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) > { > int delta = headroom - skb_headroom(skb); > + int osize = skb_end_offset(skb); > + struct sock *sk = skb->sk; > > if (WARN_ONCE(delta <= 0, > "%s is expecting an increase in the headroom", __func__)) > return skb; > > - /* pskb_expand_head() might crash, if skb is shared */ > - if (skb_shared(skb)) { > + delta = SKB_DATA_ALIGN(delta); > + /* pskb_expand_head() might crash, if skb is shared. */ > + if (skb_shared(skb) || !is_skb_wmem(skb)) { > struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); > > - if (likely(nskb)) { > - if (skb->sk) > - skb_set_owner_w(nskb, skb->sk); > - consume_skb(skb); > - } else { > - kfree_skb(skb); > - } > + if (unlikely(!nskb)) > + goto fail; > + > + if (sk) > + skb_set_owner_w(nskb, sk); > + consume_skb(skb); > skb = nskb; > } > - if (skb && > - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { > - kfree_skb(skb); > - skb = NULL; > + if (pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) > + goto fail; > + > + if (sk && skb->destructor != sock_edemux) { > + delta = skb_end_offset(skb) - osize; > + refcount_add(delta, &sk->sk_wmem_alloc); > + skb->truesize += delta; > } > return skb; > + > +fail: > + kfree_skb(skb); > + return NULL; > } > EXPORT_SYMBOL(skb_expand_head); > > diff --git a/net/core/sock.c b/net/core/sock.c > index 62627e868e03..1932755ae9ba 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -2227,6 +2227,14 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) > } > EXPORT_SYMBOL(skb_set_owner_w); > > +bool is_skb_wmem(const struct sk_buff *skb) > +{ > + return skb->destructor == sock_wfree || > + skb->destructor == __sock_wfree || > + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); > +} > +EXPORT_SYMBOL(is_skb_wmem); > + > static bool can_skb_orphan_partial(const struct sk_buff *skb) > { > #ifdef CONFIG_TLS_DEVICE > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v9] skb_expand_head() adjust skb->truesize incorrectly 2021-10-04 13:00 ` [PATCH net v9] " Vasily Averin 2021-10-04 13:14 ` Vasily Averin @ 2021-10-04 19:26 ` Eric Dumazet 2021-10-05 5:57 ` Vasily Averin 1 sibling, 1 reply; 11+ messages in thread From: Eric Dumazet @ 2021-10-04 19:26 UTC (permalink / raw) To: Vasily Averin, Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel On 10/4/21 6:00 AM, Vasily Averin wrote: > Christoph Paasch reports [1] about incorrect skb->truesize > after skb_expand_head() call in ip6_xmit. > This may happen because of two reasons: > - skb_set_owner_w() for newly cloned skb is called too early, > before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. > - pskb_expand_head() does not adjust truesize in (skb->sk) case. > In this case sk->sk_wmem_alloc should be adjusted too. > > [1] https://lkml.org/lkml/2021/8/20/1082 > > Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") > Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") > Reported-by: Christoph Paasch <christoph.paasch@gmail.com> > Signed-off-by: Vasily Averin <vvs@virtuozzo.com> > --- > v9: restored sock_edemux check > v8: clone non-wmem skb > v7 (from kuba@): > shift more magic into helpers, > follow Eric's advice and don't inherit non-wmem skbs for now > v6: fixed delta, > improved comments > v5: fixed else condition, thanks to Eric > reworked update of expanded skb, > added corresponding comments > v4: decided to use is_skb_wmem() after pskb_expand_head() call > fixed 'return (EXPRESSION);' in os_skb_wmem according to Eric Dumazet > v3: removed __pskb_expand_head(), > added is_skb_wmem() helper for skb with wmem-compatible destructors > there are 2 ways to use it: > - before pskb_expand_head(), to create skb clones > - after successfull pskb_expand_head() to change owner on extended > skb. > v2: based on patch version from Eric Dumazet, > added __pskb_expand_head() function, which can be forced > to adjust skb->truesize and sk->sk_wmem_alloc. > --- > include/net/sock.h | 1 + > net/core/skbuff.c | 35 ++++++++++++++++++++++------------- > net/core/sock.c | 8 ++++++++ > 3 files changed, 31 insertions(+), 13 deletions(-) > > diff --git a/include/net/sock.h b/include/net/sock.h > index 66a9a90f9558..a547651d46a7 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -1695,6 +1695,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force, > gfp_t priority); > void __sock_wfree(struct sk_buff *skb); > void sock_wfree(struct sk_buff *skb); > +bool is_skb_wmem(const struct sk_buff *skb); > struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size, > gfp_t priority); > void skb_orphan_partial(struct sk_buff *skb); > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 2170bea2c7de..8cb6d360cda5 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -1804,30 +1804,39 @@ EXPORT_SYMBOL(skb_realloc_headroom); > struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) > { > int delta = headroom - skb_headroom(skb); > + int osize = skb_end_offset(skb); > + struct sock *sk = skb->sk; > > if (WARN_ONCE(delta <= 0, > "%s is expecting an increase in the headroom", __func__)) > return skb; > > - /* pskb_expand_head() might crash, if skb is shared */ > - if (skb_shared(skb)) { > + delta = SKB_DATA_ALIGN(delta); > + /* pskb_expand_head() might crash, if skb is shared. */ > + if (skb_shared(skb) || !is_skb_wmem(skb)) { > struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); > > - if (likely(nskb)) { > - if (skb->sk) > - skb_set_owner_w(nskb, skb->sk); > - consume_skb(skb); > - } else { > - kfree_skb(skb); > - } > + if (unlikely(!nskb)) > + goto fail; > + > + if (sk) > + skb_set_owner_w(nskb, sk); > + consume_skb(skb); > skb = nskb; > } > - if (skb && > - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { > - kfree_skb(skb); > - skb = NULL; > + if (pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) > + goto fail; > + > + if (sk && skb->destructor != sock_edemux) { Why not re-using is_skb_wmem() here ? Testing != sock_edemux looks strange. > + delta = skb_end_offset(skb) - osize; > + refcount_add(delta, &sk->sk_wmem_alloc); > + skb->truesize += delta; > } > return skb; > + > +fail: > + kfree_skb(skb); > + return NULL; > } > EXPORT_SYMBOL(skb_expand_head); > > diff --git a/net/core/sock.c b/net/core/sock.c > index 62627e868e03..1932755ae9ba 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -2227,6 +2227,14 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) > } > EXPORT_SYMBOL(skb_set_owner_w); > > +bool is_skb_wmem(const struct sk_buff *skb) > +{ > + return skb->destructor == sock_wfree || > + skb->destructor == __sock_wfree || > + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); > +} > +EXPORT_SYMBOL(is_skb_wmem); > + This probably should be inlined. > static bool can_skb_orphan_partial(const struct sk_buff *skb) > { > #ifdef CONFIG_TLS_DEVICE > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v9] skb_expand_head() adjust skb->truesize incorrectly 2021-10-04 19:26 ` Eric Dumazet @ 2021-10-05 5:57 ` Vasily Averin 2021-10-20 16:14 ` Eric Dumazet 2021-10-20 16:18 ` Eric Dumazet 0 siblings, 2 replies; 11+ messages in thread From: Vasily Averin @ 2021-10-05 5:57 UTC (permalink / raw) To: Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel On 10/4/21 10:26 PM, Eric Dumazet wrote: > > > On 10/4/21 6:00 AM, Vasily Averin wrote: >> Christoph Paasch reports [1] about incorrect skb->truesize >> after skb_expand_head() call in ip6_xmit. >> This may happen because of two reasons: >> - skb_set_owner_w() for newly cloned skb is called too early, >> before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. >> - pskb_expand_head() does not adjust truesize in (skb->sk) case. >> In this case sk->sk_wmem_alloc should be adjusted too. >> >> [1] https://lkml.org/lkml/2021/8/20/1082 >> >> Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") >> Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") >> Reported-by: Christoph Paasch <christoph.paasch@gmail.com> >> Signed-off-by: Vasily Averin <vvs@virtuozzo.com> >> --- >> v9: restored sock_edemux check >> v8: clone non-wmem skb >> v7 (from kuba@): >> shift more magic into helpers, >> follow Eric's advice and don't inherit non-wmem skbs for now >> v6: fixed delta, >> improved comments >> v5: fixed else condition, thanks to Eric >> reworked update of expanded skb, >> added corresponding comments >> v4: decided to use is_skb_wmem() after pskb_expand_head() call >> fixed 'return (EXPRESSION);' in os_skb_wmem according to Eric Dumazet >> v3: removed __pskb_expand_head(), >> added is_skb_wmem() helper for skb with wmem-compatible destructors >> there are 2 ways to use it: >> - before pskb_expand_head(), to create skb clones >> - after successfull pskb_expand_head() to change owner on extended >> skb. >> v2: based on patch version from Eric Dumazet, >> added __pskb_expand_head() function, which can be forced >> to adjust skb->truesize and sk->sk_wmem_alloc. >> --- >> include/net/sock.h | 1 + >> net/core/skbuff.c | 35 ++++++++++++++++++++++------------- >> net/core/sock.c | 8 ++++++++ >> 3 files changed, 31 insertions(+), 13 deletions(-) >> >> diff --git a/include/net/sock.h b/include/net/sock.h >> index 66a9a90f9558..a547651d46a7 100644 >> --- a/include/net/sock.h >> +++ b/include/net/sock.h >> @@ -1695,6 +1695,7 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force, >> gfp_t priority); >> void __sock_wfree(struct sk_buff *skb); >> void sock_wfree(struct sk_buff *skb); >> +bool is_skb_wmem(const struct sk_buff *skb); >> struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size, >> gfp_t priority); >> void skb_orphan_partial(struct sk_buff *skb); >> diff --git a/net/core/skbuff.c b/net/core/skbuff.c >> index 2170bea2c7de..8cb6d360cda5 100644 >> --- a/net/core/skbuff.c >> +++ b/net/core/skbuff.c >> @@ -1804,30 +1804,39 @@ EXPORT_SYMBOL(skb_realloc_headroom); >> struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) >> { >> int delta = headroom - skb_headroom(skb); >> + int osize = skb_end_offset(skb); >> + struct sock *sk = skb->sk; >> >> if (WARN_ONCE(delta <= 0, >> "%s is expecting an increase in the headroom", __func__)) >> return skb; >> >> - /* pskb_expand_head() might crash, if skb is shared */ >> - if (skb_shared(skb)) { >> + delta = SKB_DATA_ALIGN(delta); >> + /* pskb_expand_head() might crash, if skb is shared. */ >> + if (skb_shared(skb) || !is_skb_wmem(skb)) { >> struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); >> >> - if (likely(nskb)) { >> - if (skb->sk) >> - skb_set_owner_w(nskb, skb->sk); >> - consume_skb(skb); >> - } else { >> - kfree_skb(skb); >> - } >> + if (unlikely(!nskb)) >> + goto fail; >> + >> + if (sk) >> + skb_set_owner_w(nskb, sk); >> + consume_skb(skb); >> skb = nskb; >> } >> - if (skb && >> - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { >> - kfree_skb(skb); >> - skb = NULL; >> + if (pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) >> + goto fail; >> + >> + if (sk && skb->destructor != sock_edemux) { > > Why not re-using is_skb_wmem() here ? > Testing != sock_edemux looks strange. All non-wmem skbs was cloned and then was freed already. After pskb_expand_head() call we can have: (1) either original wmem skbs (2) or cloned skbs: (2a) either without sk at all, (2b) or with sock_edemux destructor (that was set inside skb_set_owner_w() for !sk_fullsock(sk)) (2c) or with sock_wfree destructor (that was set inside skb_set_owner_w() for sk_fullsock(sk)) (2a) and (2b) do not require truesize/sk_wmem_alloc update, it was handled inside pskb_expand_head() (1) and (2c) cases are processed here. If required I can add this explanation either into patch description or as comment. Btw I just noticed that we can avoid cloning for original skbs without sk. How do you think should I do it? >> + delta = skb_end_offset(skb) - osize; >> + refcount_add(delta, &sk->sk_wmem_alloc); >> + skb->truesize += delta; >> } >> return skb; >> + >> +fail: >> + kfree_skb(skb); >> + return NULL; >> } >> EXPORT_SYMBOL(skb_expand_head); >> >> diff --git a/net/core/sock.c b/net/core/sock.c >> index 62627e868e03..1932755ae9ba 100644 >> --- a/net/core/sock.c >> +++ b/net/core/sock.c >> @@ -2227,6 +2227,14 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) >> } >> EXPORT_SYMBOL(skb_set_owner_w); >> >> +bool is_skb_wmem(const struct sk_buff *skb) >> +{ >> + return skb->destructor == sock_wfree || >> + skb->destructor == __sock_wfree || >> + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); >> +} >> +EXPORT_SYMBOL(is_skb_wmem); >> + > > This probably should be inlined. David Miller pointed me out in the comments to an early version of the patch "Please do not use inline in foo.c files, let the compiler decide." >> static bool can_skb_orphan_partial(const struct sk_buff *skb) >> { >> #ifdef CONFIG_TLS_DEVICE >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v9] skb_expand_head() adjust skb->truesize incorrectly 2021-10-05 5:57 ` Vasily Averin @ 2021-10-20 16:14 ` Eric Dumazet 2021-10-20 16:18 ` Eric Dumazet 1 sibling, 0 replies; 11+ messages in thread From: Eric Dumazet @ 2021-10-20 16:14 UTC (permalink / raw) To: Vasily Averin, Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel On 10/4/21 10:57 PM, Vasily Averin wrote: >>> >>> diff --git a/net/core/sock.c b/net/core/sock.c >>> index 62627e868e03..1932755ae9ba 100644 >>> --- a/net/core/sock.c >>> +++ b/net/core/sock.c >>> @@ -2227,6 +2227,14 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk) >>> } >>> EXPORT_SYMBOL(skb_set_owner_w); >>> >>> +bool is_skb_wmem(const struct sk_buff *skb) >>> +{ >>> + return skb->destructor == sock_wfree || >>> + skb->destructor == __sock_wfree || >>> + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); >>> +} >>> +EXPORT_SYMBOL(is_skb_wmem); >>> + >> >> This probably should be inlined. > > David Miller pointed me out in the comments to an early version of the patch > "Please do not use inline in foo.c files, let the compiler decide." > Sure, my suggestion was to move this helper in an include file, and use static inline bool .... I would not suggest add an inline in a C file, unless absolutely critical. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v9] skb_expand_head() adjust skb->truesize incorrectly 2021-10-05 5:57 ` Vasily Averin 2021-10-20 16:14 ` Eric Dumazet @ 2021-10-20 16:18 ` Eric Dumazet 2021-10-22 10:28 ` [PATCH net v10] " Vasily Averin 1 sibling, 1 reply; 11+ messages in thread From: Eric Dumazet @ 2021-10-20 16:18 UTC (permalink / raw) To: Vasily Averin, Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel On 10/4/21 10:57 PM, Vasily Averin wrote: > On 10/4/21 10:26 PM, Eric Dumazet wrote: >> >> Why not re-using is_skb_wmem() here ? >> Testing != sock_edemux looks strange. > > All non-wmem skbs was cloned and then was freed already. > After pskb_expand_head() call we can have: > (1) either original wmem skbs > (2) or cloned skbs: > (2a) either without sk at all, > (2b) or with sock_edemux destructor (that was set inside skb_set_owner_w() for !sk_fullsock(sk)) > (2c) or with sock_wfree destructor (that was set inside skb_set_owner_w() for sk_fullsock(sk)) > > (2a) and (2b) do not require truesize/sk_wmem_alloc update, it was handled inside pskb_expand_head() > (1) and (2c) cases are processed here. > > If required I can add this explanation either into patch description or as comment. > sock_edemux is one of the current destructors. New ones will be added later. We can not expect that in two or three years, at least one reviewer will remember this special case. I would prefer you list the known destructors (allow-list, instead of disallow-list) > Btw I just noticed that we can avoid cloning for original skbs without sk. > How do you think should I do it? Lets wait a bit before new features... ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net v10] skb_expand_head() adjust skb->truesize incorrectly 2021-10-20 16:18 ` Eric Dumazet @ 2021-10-22 10:28 ` Vasily Averin 2021-10-22 19:32 ` Eric Dumazet 2021-10-22 20:50 ` patchwork-bot+netdevbpf 0 siblings, 2 replies; 11+ messages in thread From: Vasily Averin @ 2021-10-22 10:28 UTC (permalink / raw) To: Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel Christoph Paasch reports [1] about incorrect skb->truesize after skb_expand_head() call in ip6_xmit. This may happen because of two reasons: - skb_set_owner_w() for newly cloned skb is called too early, before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. - pskb_expand_head() does not adjust truesize in (skb->sk) case. In this case sk->sk_wmem_alloc should be adjusted too. [1] https://lkml.org/lkml/2021/8/20/1082 Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") Reported-by: Christoph Paasch <christoph.paasch@gmail.com> Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- v10: is_skb_wmem() was moved into separate header (it depends on net/tcp.h) use it after pskb_expand_head() insted of strange sock_edemux check v9: restored sock_edemux check v8: clone non-wmem skb v7 (from kuba@): shift more magic into helpers, follow Eric's advice and don't inherit non-wmem skbs for now v6: fixed delta, improved comments v5: fixed else condition, thanks to Eric reworked update of expanded skb, added corresponding comments v4: decided to use is_skb_wmem() after pskb_expand_head() call fixed 'return (EXPRESSION);' in os_skb_wmem according to Eric Dumazet v3: removed __pskb_expand_head(), added is_skb_wmem() helper for skb with wmem-compatible destructors there are 2 ways to use it: - before pskb_expand_head(), to create skb clones - after successfull pskb_expand_head() to change owner on extended skb. v2: based on patch version from Eric Dumazet, added __pskb_expand_head() function, which can be forced to adjust skb->truesize and sk->sk_wmem_alloc. net/core/skbuff.c | 36 +++++++++++++++++++++++------------- net/core/sock_destructor.h | 12 ++++++++++++ 2 files changed, 35 insertions(+), 13 deletions(-) create mode 100644 net/core/sock_destructor.h diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 2170bea2c7de..fe9358437380 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -80,6 +80,7 @@ #include <linux/indirect_call_wrapper.h> #include "datagram.h" +#include "sock_destructor.h" struct kmem_cache *skbuff_head_cache __ro_after_init; static struct kmem_cache *skbuff_fclone_cache __ro_after_init; @@ -1804,30 +1805,39 @@ EXPORT_SYMBOL(skb_realloc_headroom); struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) { int delta = headroom - skb_headroom(skb); + int osize = skb_end_offset(skb); + struct sock *sk = skb->sk; if (WARN_ONCE(delta <= 0, "%s is expecting an increase in the headroom", __func__)) return skb; - /* pskb_expand_head() might crash, if skb is shared */ - if (skb_shared(skb)) { + delta = SKB_DATA_ALIGN(delta); + /* pskb_expand_head() might crash, if skb is shared. */ + if (skb_shared(skb) || !is_skb_wmem(skb)) { struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); - if (likely(nskb)) { - if (skb->sk) - skb_set_owner_w(nskb, skb->sk); - consume_skb(skb); - } else { - kfree_skb(skb); - } + if (unlikely(!nskb)) + goto fail; + + if (sk) + skb_set_owner_w(nskb, sk); + consume_skb(skb); skb = nskb; } - if (skb && - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { - kfree_skb(skb); - skb = NULL; + if (pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) + goto fail; + + if (sk && is_skb_wmem(skb)) { + delta = skb_end_offset(skb) - osize; + refcount_add(delta, &sk->sk_wmem_alloc); + skb->truesize += delta; } return skb; + +fail: + kfree_skb(skb); + return NULL; } EXPORT_SYMBOL(skb_expand_head); diff --git a/net/core/sock_destructor.h b/net/core/sock_destructor.h new file mode 100644 index 000000000000..2f396e6bfba5 --- /dev/null +++ b/net/core/sock_destructor.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#ifndef _NET_CORE_SOCK_DESTRUCTOR_H +#define _NET_CORE_SOCK_DESTRUCTOR_H +#include <net/tcp.h> + +static inline bool is_skb_wmem(const struct sk_buff *skb) +{ + return skb->destructor == sock_wfree || + skb->destructor == __sock_wfree || + (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree); +} +#endif -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net v10] skb_expand_head() adjust skb->truesize incorrectly 2021-10-22 10:28 ` [PATCH net v10] " Vasily Averin @ 2021-10-22 19:32 ` Eric Dumazet 2021-10-22 20:50 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 11+ messages in thread From: Eric Dumazet @ 2021-10-22 19:32 UTC (permalink / raw) To: Vasily Averin, Eric Dumazet, Jakub Kicinski Cc: netdev, David S. Miller, Hideaki YOSHIFUJI, David Ahern, Julian Wiedmann, Christoph Paasch, linux-kernel, kernel On 10/22/21 3:28 AM, Vasily Averin wrote: > Christoph Paasch reports [1] about incorrect skb->truesize > after skb_expand_head() call in ip6_xmit. > This may happen because of two reasons: > - skb_set_owner_w() for newly cloned skb is called too early, > before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. > - pskb_expand_head() does not adjust truesize in (skb->sk) case. > In this case sk->sk_wmem_alloc should be adjusted too. > > [1] https://lkml.org/lkml/2021/8/20/1082 > > Fixes: f1260ff15a71 ("skbuff: introduce skb_expand_head()") > Fixes: 2d85a1b31dde ("ipv6: ip6_finish_output2: set sk into newly allocated nskb") > Reported-by: Christoph Paasch <christoph.paasch@gmail.com> > Signed-off-by: Vasily Averin <vvs@virtuozzo.com> > --- > v10: is_skb_wmem() was moved into separate header (it depends on net/tcp.h) > use it after pskb_expand_head() insted of strange sock_edemux check SGTM, thanks ! Reviewed-by: Eric Dumazet <edumazet@google.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net v10] skb_expand_head() adjust skb->truesize incorrectly 2021-10-22 10:28 ` [PATCH net v10] " Vasily Averin 2021-10-22 19:32 ` Eric Dumazet @ 2021-10-22 20:50 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 11+ messages in thread From: patchwork-bot+netdevbpf @ 2021-10-22 20:50 UTC (permalink / raw) To: Vasily Averin Cc: eric.dumazet, kuba, netdev, davem, yoshfuji, dsahern, jwi, christoph.paasch, linux-kernel, kernel Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Fri, 22 Oct 2021 13:28:37 +0300 you wrote: > Christoph Paasch reports [1] about incorrect skb->truesize > after skb_expand_head() call in ip6_xmit. > This may happen because of two reasons: > - skb_set_owner_w() for newly cloned skb is called too early, > before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. > - pskb_expand_head() does not adjust truesize in (skb->sk) case. > In this case sk->sk_wmem_alloc should be adjusted too. > > [...] Here is the summary with links: - [net,v10] skb_expand_head() adjust skb->truesize incorrectly https://git.kernel.org/netdev/net/c/7f678def99d2 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-22 20:50 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20210920111259.18f9cc01@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> 2021-09-20 21:41 ` [PATCH net v8] skb_expand_head() adjust skb->truesize incorrectly Vasily Averin 2021-09-21 5:21 ` Vasily Averin 2021-10-04 13:00 ` [PATCH net v9] " Vasily Averin 2021-10-04 13:14 ` Vasily Averin 2021-10-04 19:26 ` Eric Dumazet 2021-10-05 5:57 ` Vasily Averin 2021-10-20 16:14 ` Eric Dumazet 2021-10-20 16:18 ` Eric Dumazet 2021-10-22 10:28 ` [PATCH net v10] " Vasily Averin 2021-10-22 19:32 ` Eric Dumazet 2021-10-22 20:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).