From patchwork Wed Nov 29 17:15:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markos Chandras X-Patchwork-Id: 842673 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) 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 3yn6dn5vs8z9sBZ for ; Thu, 30 Nov 2017 04:16:45 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 32947CB7; Wed, 29 Nov 2017 17:16:44 +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 544CDC84 for ; Wed, 29 Nov 2017 17:16:43 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7B5B5517 for ; Wed, 29 Nov 2017 17:16:42 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 1AE59ACA6; Wed, 29 Nov 2017 17:16:40 +0000 (UTC) From: Markos Chandras To: dev@openvswitch.org Date: Wed, 29 Nov 2017 17:15:29 +0000 Message-Id: <20171129171529.16132-1-mchandras@suse.de> X-Mailer: git-send-email 2.15.0 In-Reply-To: <20171129165150.GF13883@ovn.org> References: <20171129165150.GF13883@ovn.org> X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH 2.5] ofp-util: Fix memory leaks on error cases in ofputil_decode_group_mod(). 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: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: Ben Pfaff Found by libFuzzer. Reported-by: Bhargava Shastry Signed-off-by: Ben Pfaff Acked-by: Justin Pettit Signed-off-by: Markos Chandras --- Hi Ben, It seems that this patch applies with some fixups on branch-2.5 so it should be good for inclusion. --- lib/ofp-util.c | 88 ++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 7b4d62b99..a29c47370 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -8039,6 +8039,7 @@ ofputil_pull_ofp11_buckets(struct ofpbuf *msg, size_t buckets_length, if (!ob) { VLOG_WARN_RL(&bad_ofmsg_rl, "buckets end with %"PRIuSIZE" leftover bytes", buckets_length); + ofputil_bucket_list_destroy(buckets); return OFPERR_OFPGMFC_BAD_BUCKET; } @@ -8046,11 +8047,13 @@ ofputil_pull_ofp11_buckets(struct ofpbuf *msg, size_t buckets_length, if (ob_len < sizeof *ob) { VLOG_WARN_RL(&bad_ofmsg_rl, "OpenFlow message bucket length " "%"PRIuSIZE" is not valid", ob_len); + ofputil_bucket_list_destroy(buckets); return OFPERR_OFPGMFC_BAD_BUCKET; } else if (ob_len > buckets_length) { VLOG_WARN_RL(&bad_ofmsg_rl, "OpenFlow message bucket length " "%"PRIuSIZE" exceeds remaining buckets data size %"PRIuSIZE, ob_len, buckets_length); + ofputil_bucket_list_destroy(buckets); return OFPERR_OFPGMFC_BAD_BUCKET; } buckets_length -= ob_len; @@ -8769,6 +8772,7 @@ ofputil_pull_ofp11_group_mod(struct ofpbuf *msg, enum ofp_version ofp_version, && gm->command == OFPGC11_DELETE && !list_is_empty(&gm->buckets)) { error = OFPERR_OFPGMFC_INVALID_GROUP; + ofputil_bucket_list_destroy(&gm->buckets); } return error; @@ -8836,44 +8840,9 @@ ofputil_pull_ofp15_group_mod(struct ofpbuf *msg, enum ofp_version ofp_version, return error; } -/* Converts OpenFlow group mod message 'oh' into an abstract group mod in - * 'gm'. Returns 0 if successful, otherwise an OpenFlow error code. */ -enum ofperr -ofputil_decode_group_mod(const struct ofp_header *oh, - struct ofputil_group_mod *gm) +static enum ofperr +ofputil_check_group_mod(const struct ofputil_group_mod *gm) { - enum ofp_version ofp_version = oh->version; - struct ofpbuf msg; - struct ofputil_bucket *bucket; - enum ofperr err; - - ofpbuf_use_const(&msg, oh, ntohs(oh->length)); - ofpraw_pull_assert(&msg); - - ofputil_init_group_properties(&gm->props); - - switch (ofp_version) - { - case OFP11_VERSION: - case OFP12_VERSION: - case OFP13_VERSION: - case OFP14_VERSION: - err = ofputil_pull_ofp11_group_mod(&msg, ofp_version, gm); - break; - - case OFP15_VERSION: - err = ofputil_pull_ofp15_group_mod(&msg, ofp_version, gm); - break; - - case OFP10_VERSION: - default: - OVS_NOT_REACHED(); - } - - if (err) { - return err; - } - switch (gm->type) { case OFPGT11_INDIRECT: if (!list_is_singleton(&gm->buckets)) { @@ -8903,6 +8872,7 @@ ofputil_decode_group_mod(const struct ofp_header *oh, return OFPERR_OFPGMFC_BAD_COMMAND; } + struct ofputil_bucket *bucket; LIST_FOR_EACH (bucket, list_node, &gm->buckets) { if (bucket->weight && gm->type != OFPGT11_SELECT) { return OFPERR_OFPGMFC_INVALID_GROUP; @@ -8930,6 +8900,50 @@ ofputil_decode_group_mod(const struct ofp_header *oh, return 0; } +/* Converts OpenFlow group mod message 'oh' into an abstract group mod in + * 'gm'. Returns 0 if successful, otherwise an OpenFlow error code. */ +enum ofperr +ofputil_decode_group_mod(const struct ofp_header *oh, + struct ofputil_group_mod *gm) +{ + enum ofp_version ofp_version = oh->version; + struct ofpbuf msg; + enum ofperr err; + + ofpbuf_use_const(&msg, oh, ntohs(oh->length)); + ofpraw_pull_assert(&msg); + + ofputil_init_group_properties(&gm->props); + + switch (ofp_version) + { + case OFP11_VERSION: + case OFP12_VERSION: + case OFP13_VERSION: + case OFP14_VERSION: + err = ofputil_pull_ofp11_group_mod(&msg, ofp_version, gm); + break; + + case OFP15_VERSION: + err = ofputil_pull_ofp15_group_mod(&msg, ofp_version, gm); + break; + + case OFP10_VERSION: + default: + OVS_NOT_REACHED(); + } + + if (err) { + return err; + } + + err = ofputil_check_group_mod(gm); + if (err) { + ofputil_uninit_group_mod(gm); + } + return err; +} + /* Parse a queue status request message into 'oqsr'. * Returns 0 if successful, otherwise an OFPERR_* number. */ enum ofperr