From mboxrd@z Thu Jan 1 00:00:00 1970 From: Calvin Owens Subject: Re: [PATCH] ipvs: Keep skb->sk when allocating headroom on tunnel xmit Date: Fri, 7 Nov 2014 14:12:13 -0800 Message-ID: <545D43BD.8030203@fb.com> References: <1415147860-11389-1-git-send-email-calvinowens@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: Simon Horman , Wensong Zhang , , , , , To: Julian Anastasov Return-path: In-Reply-To: Sender: lvs-devel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 11/05/2014 01:21 AM, Julian Anastasov wrote: > > Hello, > > On Tue, 4 Nov 2014, Calvin Owens wrote: > >> ip_vs_prepare_tunneled_skb() ignores ->sk when allocating a new >> skb, either unconditionally setting ->sk to NULL or allowing >> the uninitialized ->sk from a newly allocated skb to leak through >> to the caller. >> >> This patch properly copies ->sk and increments its reference count. >> >> Signed-off-by: Calvin Owens > > Good catch. Please, extend your patch to > fix also the second place that has such error, > ip_vs_tunnel_xmit_v6. This call is missing from long time, > it was not needed. But commits that allow skb->sk (local > clients) already need it, eg. I'm not sure where exactly you mean: ip_vs_tunnel_xmit_v6() calls ip_vs_prepare_tunneled_skb() to do the allocation, so this patch covers that case. In older versions of the kernel, ip_vs_tunnel_xmit_v6() does it directly, could that be what you're looking at? > - f2428ed5e7bc89c7 ("ipvs: load balance ipv6 connections from a local > process"), 2.6.28 > - 4856c84c1358b798 ("ipvs: load balance IPv4 connections from a local > process"), 2.6.28 > >> --- >> net/netfilter/ipvs/ip_vs_xmit.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c >> index 437a366..bd90bf8 100644 >> --- a/net/netfilter/ipvs/ip_vs_xmit.c >> +++ b/net/netfilter/ipvs/ip_vs_xmit.c >> @@ -846,6 +846,8 @@ ip_vs_prepare_tunneled_skb(struct sk_buff *skb, int skb_af, >> new_skb = skb_realloc_headroom(skb, max_headroom); >> if (!new_skb) >> goto error; >> + if (skb->sk) >> + skb_set_owner_w(new_skb, skb->sk); >> consume_skb(skb); >> skb = new_skb; >> } > > Regards > > -- > Julian Anastasov >