Commit 7f678def authored by Vasily Averin's avatar Vasily Averin Committed by Jakub Kicinski
Browse files

skb_expand_head() adjust skb->truesize incorrectly 

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: f1260ff1 ("skbuff: introduce skb_expand_head()")
Fixes: 2d85a1b3 ("ipv6: ip6_finish_output2: set sk into newly allocated nskb")
Reported-by: default avatarChristoph Paasch <christoph.paasch@gmail.com>
Signed-off-by: default avatarVasily Averin <vvs@virtuozzo.com>
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Link: https://lore.kernel.org/r/644330dd-477e-0462-83bf-9f514c41edd1@virtuozzo.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 7fcb1c95

net/core/skbuff.c

+23 −13
Original line number Diff line number Diff line
@@ -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);

		if (unlikely(!nskb))

			goto fail;



		if (sk)

			skb_set_owner_w(nskb, sk);

		consume_skb(skb);

		} else {

			kfree_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);

net/core/sock_destructor.h

0 → 100644
+12 −0
Original line number Diff line number Diff line 
/* 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