From patchwork Tue Nov 5 11:02:13 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiri Pirko X-Patchwork-Id: 288476 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id B9B712C0185 for ; Tue, 5 Nov 2013 22:02:35 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754330Ab3KELC2 (ORCPT ); Tue, 5 Nov 2013 06:02:28 -0500 Received: from mail-ee0-f51.google.com ([74.125.83.51]:33681 "EHLO mail-ee0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754184Ab3KELC0 (ORCPT ); Tue, 5 Nov 2013 06:02:26 -0500 Received: by mail-ee0-f51.google.com with SMTP id t10so1711328eei.10 for ; Tue, 05 Nov 2013 03:02:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=xkmAuynzXV1zSgbbatH+muHVqr5OmhG2sIPK6FTM8Xw=; b=aSJXGE05uUvYMfw3229HFFQyzOR5fFIlyxjBuwnfRAW8kHXYdqeK/i9WavqUPWtNDZ LzRZ/azJqRYDUgDVdE7996IDikXTMpeGRVAwkxHxcdvC4OXdIaU6pv/KEGYieE7swWu0 hjm5GOijyxWyaBUHx/QNAvowCpJp1JiN2xXeRRYkDhiYmDsqjLC3Z01L+yPN1FvvGbN0 ikqSZw9MPVuOQR0gDt/0e1/dpxUeCui3Lz8G/kttUpOFQLYjpMdJPMBiMFBH4F38Lk0l 11hhQNkI6DbrGPYCkcuoW788H1dNlIYaCOYk125qKeR87DxxbyaMlHPPSQx1k5H7SotM 76Ug== X-Gm-Message-State: ALoCoQnrz8j8Q+k9eqU6GVRwf3fZjQZ0r75U0Kn7VmPQIg2F2swYgBy5q2HCgKi3Noqj6mg60YbS X-Received: by 10.14.178.67 with SMTP id e43mr10020178eem.59.1383649344379; Tue, 05 Nov 2013 03:02:24 -0800 (PST) Received: from localhost (sun-0.pirko.cz. [84.16.102.25]) by mx.google.com with ESMTPSA id a6sm58756140eei.10.2013.11.05.03.02.22 for (version=TLSv1.2 cipher=AES128-GCM-SHA256 bits=128/128); Tue, 05 Nov 2013 03:02:23 -0800 (PST) From: Jiri Pirko To: netdev@vger.kernel.org Cc: davem@davemloft.net, pablo@netfilter.org, netfilter-devel@vger.kernel.org, yoshfuji@linux-ipv6.org, kadlec@blackhole.kfki.hu, kaber@trash.net, mleitner@redhat.com, kuznet@ms2.inr.ac.ru, jmorris@namei.org, wensong@linux-vs.org, horms@verge.net.au, ja@ssi.bg, edumazet@google.com, pshelar@nicira.com, jasowang@redhat.com, alexander.h.duyck@intel.com, coreteam@netfilter.org, fw@strlen.de Subject: [patch net-next 3/3] fix skb_morph to preserve skb->sk and skb->destructor pointers Date: Tue, 5 Nov 2013 12:02:13 +0100 Message-Id: <1383649333-6321-4-git-send-email-jiri@resnulli.us> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1383649333-6321-1-git-send-email-jiri@resnulli.us> References: <1383649333-6321-1-git-send-email-jiri@resnulli.us> Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Currently __skb_clone sets skb->sk and skb->destructor to NULL. This is not right for skb_morph use case because skb->sk may be previously set (e. g. by xt_TPROXY). Also, during skb_morph the destructor should not be called. It might be previously set, e. g. by xt_TPROXY to sock_edemux, and that would cause put sk while skb is still in flight. This patch fixes these. Signed-off-by: Jiri Pirko Signed-off-by: Marcelo Ricardo Leitner --- net/core/skbuff.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 3735fad..21b320e 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -515,7 +515,7 @@ static void skb_free_head(struct sk_buff *skb) kfree(skb->head); } -static void skb_release_data(struct sk_buff *skb) +static void __skb_release_data(struct sk_buff *skb) { if (!skb->cloned || !atomic_sub_return(skb->nohdr ? (1 << SKB_DATAREF_SHIFT) + 1 : 1, @@ -579,16 +579,12 @@ static void kfree_skbmem(struct sk_buff *skb) } } -static void skb_release_head_state(struct sk_buff *skb) +static void __skb_release_head_state(struct sk_buff *skb) { skb_dst_drop(skb); #ifdef CONFIG_XFRM secpath_put(skb->sp); #endif - if (skb->destructor) { - WARN_ON(in_irq()); - skb->destructor(skb); - } #if IS_ENABLED(CONFIG_NF_CONNTRACK) nf_conntrack_put(skb->nfct); #endif @@ -607,12 +603,19 @@ static void skb_release_head_state(struct sk_buff *skb) #endif } -/* Free everything but the sk_buff shell. */ -static void skb_release_all(struct sk_buff *skb) +static void skb_release_head_state(struct sk_buff *skb) +{ + if (skb->destructor) { + WARN_ON(in_irq()); + skb->destructor(skb); + } + __skb_release_head_state(skb); +} + +static void skb_release_data(struct sk_buff *skb) { - skb_release_head_state(skb); if (likely(skb->head)) - skb_release_data(skb); + __skb_release_data(skb); } /** @@ -626,7 +629,8 @@ static void skb_release_all(struct sk_buff *skb) void __kfree_skb(struct sk_buff *skb) { - skb_release_all(skb); + skb_release_head_state(skb); + skb_release_data(skb); kfree_skbmem(skb); } EXPORT_SYMBOL(__kfree_skb); @@ -761,12 +765,11 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old) * You should not add any new code to this function. Add it to * __copy_skb_header above instead. */ -static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) +static struct sk_buff *___skb_clone(struct sk_buff *n, struct sk_buff *skb) { #define C(x) n->x = skb->x n->next = n->prev = NULL; - n->sk = NULL; __copy_skb_header(n, skb); C(len); @@ -775,7 +778,6 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) n->hdr_len = skb->nohdr ? skb_headroom(skb) : skb->hdr_len; n->cloned = 1; n->nohdr = 0; - n->destructor = NULL; C(tail); C(end); C(head); @@ -791,6 +793,13 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) #undef C } +static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) +{ + n->sk = NULL; + n->destructor = NULL; + return ___skb_clone(n, skb); +} + /** * skb_morph - morph one skb into another * @dst: the skb to receive the contents @@ -803,8 +812,9 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) */ struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src) { - skb_release_all(dst); - return __skb_clone(dst, src); + __skb_release_head_state(dst); + skb_release_data(dst); + return ___skb_clone(dst, src); } EXPORT_SYMBOL_GPL(skb_morph); @@ -1107,7 +1117,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail, if (skb_has_frag_list(skb)) skb_clone_fraglist(skb); - skb_release_data(skb); + __skb_release_data(skb); } else { skb_free_head(skb); }