From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752812AbbI3OmY (ORCPT ); Wed, 30 Sep 2015 10:42:24 -0400 Received: from mail-la0-f51.google.com ([209.85.215.51]:36357 "EHLO mail-la0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752307AbbI3OmV (ORCPT ); Wed, 30 Sep 2015 10:42:21 -0400 Subject: Re: [PATCH net 3/7] openvswitch: Fix skb leak in ovs_fragment() To: Joe Stringer , netdev@vger.kernel.org, pshelar@nicira.com References: <1443566380-22640-1-git-send-email-joestringer@nicira.com> <1443566380-22640-4-git-send-email-joestringer@nicira.com> Cc: linux-kernel@vger.kernel.org From: Sergei Shtylyov Organization: Cogent Embedded Message-ID: <560BF4C9.7080509@cogentembedded.com> Date: Wed, 30 Sep 2015 17:42:17 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1443566380-22640-4-git-send-email-joestringer@nicira.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello. On 09/30/2015 01:39 AM, Joe Stringer wrote: > If ovs_fragment() was unable to fragment the skb due to an L2 header > that exceeds the supported length, skbs would be leaked. Fix the bug. > > Fixes: 7f8a436 "openvswitch: Add conntrack action" > Signed-off-by: Joe Stringer > --- > net/openvswitch/actions.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c > index e23a61c..e1afbd1 100644 > --- a/net/openvswitch/actions.c > +++ b/net/openvswitch/actions.c [...] > @@ -728,8 +727,14 @@ static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru, > WARN_ONCE(1, "Failed fragment ->%s: eth=%04x, MRU=%d, MTU=%d.", > ovs_vport_name(vport), ntohs(ethertype), mru, > vport->dev->mtu); > - kfree_skb(skb); > + goto out; > } > + > + skb = NULL; I'd just return here. > + > +out: > + if (skb) > + kfree_skb(skb); kfree_skb() checks for NULL. [...] MBR, Sergei