From patchwork Wed Jun 21 18:06:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shashank Ram X-Patchwork-Id: 779048 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wtCMX2xcmz9ryr for ; Thu, 22 Jun 2017 04:06:31 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=onevmw.onmicrosoft.com header.i=@onevmw.onmicrosoft.com header.b="efTaOyqh"; dkim-atps=neutral Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 328C088A; Wed, 21 Jun 2017 18:06:28 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 59177707 for ; Wed, 21 Jun 2017 18:06:27 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0048.outbound.protection.outlook.com [104.47.32.48]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 74602AB for ; Wed, 21 Jun 2017 18:06:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=onevmw.onmicrosoft.com; s=selector1-vmware-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=H5Mc3c5HiMXa8n/PWCtjZqiDIDCvH+YQPjKPjSVavLY=; b=efTaOyqhizWXY9730IEMeqPQQFwubYHjWKqpLelPl5ypYbTx/v65IdhZhZAkwvbovscL2b4ezEFsHpkkwy7x7Jxh503o6/0fuwt5d1nFEPLCcbCE0OeBbz5asMV74MFLvOuj1lEtGwnEOu7h+qVNcO9J/vRYUkz49wgHzUB48gw= Received: from BY2PR0501MB2119.namprd05.prod.outlook.com (10.163.198.17) by BY2PR0501MB1670.namprd05.prod.outlook.com (10.163.154.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1199.6; Wed, 21 Jun 2017 18:06:24 +0000 Received: from BY2PR0501MB2119.namprd05.prod.outlook.com ([10.163.198.17]) by BY2PR0501MB2119.namprd05.prod.outlook.com ([10.163.198.17]) with mapi id 15.01.1199.012; Wed, 21 Jun 2017 18:06:23 +0000 From: Shashank Ram To: Sairam Venugopal , "dev@openvswitch.org" Thread-Topic: [ovs-dev] [PATCH v2] datapath-windows: Fix potential memory leak while creating conntrack entry Thread-Index: AQHS6rEio6fw88V1KEiejzKk905OyqIvmB5L Date: Wed, 21 Jun 2017 18:06:23 +0000 Message-ID: References: <20170621170857.18804-1-vsairam@vmware.com> In-Reply-To: <20170621170857.18804-1-vsairam@vmware.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: vmware.com; dkim=none (message not signed) header.d=none; vmware.com; dmarc=none action=none header.from=vmware.com; x-originating-ip: [208.91.1.34] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; BY2PR0501MB1670; 20:NWnnsRS+pewO/NWIabOT7tjVPABvSFO6R29Cazz+kBaIT0zqFn5jEc11hCAKc4qfOtJpwfRExk8QnyYFdEZPSEEQFOuTwy/GZUZHJuccqZzFSvBdU0b3GzVSCSq1MXZ85V2gjKrGfyDr6zRRkuznQpFUTAwG/BoVbaELX6+y9Og= x-forefront-antispam-report: SFV:SKI; SCL:-1SFV:NSPM; SFS:(10009020)(6009001)(39860400002)(39450400003)(39840400002)(39850400002)(39400400002)(39410400002)(377454003)(478600001)(6506006)(3280700002)(6436002)(33656002)(305945005)(189998001)(53936002)(2950100002)(86362001)(14454004)(53546010)(6246003)(3660700001)(38730400002)(7696004)(66066001)(2501003)(2900100001)(7736002)(81166006)(50986999)(54356999)(76176999)(8936002)(6116002)(102836003)(3846002)(2906002)(77096006)(5660300001)(9686003)(74316002)(55016002)(122556002)(99286003)(229853002)(25786009)(586874002)(21314002); DIR:OUT; SFP:1101; SCL:1; SRVR:BY2PR0501MB1670; H:BY2PR0501MB2119.namprd05.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en; x-ms-office365-filtering-correlation-id: 075c0e64-a303-4694-6c3b-08d4b8d03a3c x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254075)(201703131423075)(201703031133081); SRVR:BY2PR0501MB1670; x-ms-traffictypediagnostic: BY2PR0501MB1670: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(61668805478150)(216315784871565); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(8121501046)(5005006)(93006095)(93001095)(3002001)(100000703101)(100105400095)(10201501046)(6041248)(20161123558100)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123562025)(20161123555025)(20161123564025)(20161123560025)(6072148)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:BY2PR0501MB1670; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:BY2PR0501MB1670; x-forefront-prvs: 0345CFD558 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: vmware.com X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Jun 2017 18:06:23.3641 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR0501MB1670 X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Fix potential memory leak while creating conntrack entry X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c index 68ed395..b010484 100644 --- a/datapath-windows/ovsext/Conntrack.c +++ b/datapath-windows/ovsext/Conntrack.c @@ -214,87 +214,78 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx, BOOLEAN *entryCreated) { POVS_CT_ENTRY entry = NULL; - *entryCreated = FALSE; UINT32 state = 0; + POVS_CT_ENTRY parentEntry; PNET_BUFFER_LIST curNbl = fwdCtx->curNbl; - switch (ipProto) - { - case IPPROTO_TCP: - { - TCPHdr tcpStorage; - const TCPHdr *tcp; - tcp = OvsGetTcp(curNbl, l4Offset, &tcpStorage); - if (!OvsConntrackValidateTcpPacket(tcp)) { - goto invalid; - } - - state |= OVS_CS_F_NEW; - POVS_CT_ENTRY parentEntry; - parentEntry = OvsCtRelatedLookup(ctx->key, currentTime); - if (parentEntry != NULL) { - state |= OVS_CS_F_RELATED; - } - if (commit) { - entry = OvsConntrackCreateTcpEntry(tcp, curNbl, currentTime); - if (!entry) { - return NULL; - } + *entryCreated = FALSE; + state |= OVS_CS_F_NEW; - /* Set parent entry for related FTP connections */ - entry->parent = parentEntry; + parentEntry = OvsCtRelatedLookup(ctx->key, currentTime); + if (parentEntry != NULL) { + state |= OVS_CS_F_RELATED; + } - *entryCreated = TRUE; - } + switch (ipProto) { + case IPPROTO_TCP: + { + TCPHdr tcpStorage; + const TCPHdr *tcp; + tcp = OvsGetTcp(curNbl, l4Offset, &tcpStorage); + if (!OvsConntrackValidateTcpPacket(tcp)) { + state = OVS_CS_F_INVALID; break; } - case IPPROTO_ICMP: - { - ICMPHdr storage; - const ICMPHdr *icmp; - icmp = OvsGetIcmp(curNbl, l4Offset, &storage); - if (!OvsConntrackValidateIcmpPacket(icmp)) { - goto invalid; - } - state |= OVS_CS_F_NEW; - if (commit) { - entry = OvsConntrackCreateIcmpEntry(currentTime); - if (entry) { - /* XXX Add support for ICMP-Related */ - entry->parent = NULL; - } - *entryCreated = TRUE; - } + if (commit) { + entry = OvsConntrackCreateTcpEntry(tcp, curNbl, currentTime); + } + break; + } + case IPPROTO_ICMP: + { + ICMPHdr storage; + const ICMPHdr *icmp; + icmp = OvsGetIcmp(curNbl, l4Offset, &storage); + if (!OvsConntrackValidateIcmpPacket(icmp)) { + state = OVS_CS_F_INVALID; break; } - case IPPROTO_UDP: - { - state |= OVS_CS_F_NEW; - if (commit) { - entry = OvsConntrackCreateOtherEntry(currentTime); - if (entry) { - /* Default UDP related to NULL until TFTP is supported */ - entry->parent = NULL; - } + + if (commit) { + entry = OvsConntrackCreateIcmpEntry(currentTime); + } + break; + } + case IPPROTO_UDP: + { + if (commit) { + entry = OvsConntrackCreateOtherEntry(currentTime); + } + break; + } + default: >> nit: Missing parenthesis for default case + state = OVS_CS_F_INVALID; + break; + } + + if (state != OVS_CS_F_INVALID && commit) { + if (entry) { + entry->parent = parentEntry; + if (OvsCtAddEntry(entry, ctx, natInfo, currentTime)) { *entryCreated = TRUE; + } else { + /* Unable to add entry to the list */ + OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG); + state = OVS_CS_F_INVALID; + entry = NULL; } - break; + } else { + /* OvsAllocateMemoryWithTag returned NULL; treat as invalid */ + state = OVS_CS_F_INVALID; } - default: - goto invalid; } - if (commit && !entry) { - return NULL; - } - if (entry && !OvsCtAddEntry(entry, ctx, natInfo, currentTime)) { - return NULL; - } - OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); - return entry; -invalid: - state |= OVS_CS_F_INVALID; OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);