From patchwork Thu Mar 16 16:36:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Raphael Tiovalen X-Patchwork-Id: 1757942 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=TdsgAZjM; dkim-atps=neutral Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4PctGF5bcYz2470 for ; Fri, 17 Mar 2023 03:37:25 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 2B30141AFB; Thu, 16 Mar 2023 16:37:22 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 2B30141AFB Authentication-Results: smtp4.osuosl.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=TdsgAZjM X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ElaS2outvfqY; Thu, 16 Mar 2023 16:37:19 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 6E69A41999; Thu, 16 Mar 2023 16:37:18 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 6E69A41999 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 32D7BC0071; Thu, 16 Mar 2023 16:37:18 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 59AC8C0032 for ; Thu, 16 Mar 2023 16:37:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 1F65E4038B for ; Thu, 16 Mar 2023 16:37:17 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 1F65E4038B Authentication-Results: smtp2.osuosl.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=TdsgAZjM X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hJTMtWf5ESPy for ; Thu, 16 Mar 2023 16:37:14 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org B2035400E2 Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by smtp2.osuosl.org (Postfix) with ESMTPS id B2035400E2 for ; Thu, 16 Mar 2023 16:37:14 +0000 (UTC) Received: by mail-pj1-x102a.google.com with SMTP id d13so2233617pjh.0 for ; Thu, 16 Mar 2023 09:37:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678984633; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=Un9jUMfPLhGvWXiRlKKn48V7gFeaoPzhr+FhaUYH2pg=; b=TdsgAZjMyaTV5gJmbU4x8w0JVecYGMvk7B0fCrRvqW8OTuwNCIJX3Ol8YGLFygeyj8 oOkFtX8eZLbyllXwgDsXFdFLwTSC7bEbW1RQH7dBYZF4GjYLDAZ5M0zVW6/Rkzeww8+v E2pvlb/0D8HGPSMuMRy4vAJ1ZmhFd2VZ0LkON6o6OqJa9n2Lxb0xK625Gy9mClC7MEbM 7oEF6h4wbY71l+y3F91eMxEaa6fcy+pXTcNNgE8yLwYIm3FE1kwT65ZDLbXaent0tmwY 6tUEKootHRyCEyXr55qJ0Dxw/t7euVccgVi5Rw0nkfJjqISkvaxl5+49wfzE8c//43MI +LiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678984633; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Un9jUMfPLhGvWXiRlKKn48V7gFeaoPzhr+FhaUYH2pg=; b=IBL64eT91xh11rqKSB+ON3+bTgHsT1rZjMWyTFUh+pGmp4vp7oXxF6W6OJOcwNmkxE RXvo9TXfK+ud1f4VlbIN/NAHy5G/i/LrcbFMooP5d3De4PI1V4r8nsKEvf4IJu9ASNry 2r25Ne6feGDeqy5BY5EcEmEifPSaWQAbusyLiS6cYdYsKRXaG87PZSc9DEzz1bdcsr2J Sy4oDUa6Tew7YF8thD7EC+m/7TrB5r8znS+5N+pJBE1A6nE6XL3OsSkbr7d23tlB7tAM BDTZ2CHrwHnX9yqDKMbN8qnmjRoIAowiSY8DyQ25jChBgMGHh97j2jFsttRtohRGVUAx D7TQ== X-Gm-Message-State: AO0yUKXZfP4miSAK+6U/u9Me7BXD8DYfz9Ze9Gqd7Rgrr3ilPUB11H1y GNsSLH/YY/EqwS3pJ2ZK3GI+jirDmZocbA== X-Google-Smtp-Source: AK7set9rpN+y+3O8Ud5h92IVXeIA5UnQKWiv66YCqpLcGzlK3wIF8+Z1sPGvxqyzfBSSWPUiNufc0w== X-Received: by 2002:a17:902:f545:b0:19c:eb9a:7712 with SMTP id h5-20020a170902f54500b0019ceb9a7712mr5445415plf.1.1678984633080; Thu, 16 Mar 2023 09:37:13 -0700 (PDT) Received: from JRT-PC.lan ([103.252.200.22]) by smtp.gmail.com with ESMTPSA id o11-20020a170902d4cb00b0019f9fd5c24asm5823052plg.207.2023.03.16.09.37.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Mar 2023 09:37:12 -0700 (PDT) From: James Raphael Tiovalen To: dev@openvswitch.org Date: Fri, 17 Mar 2023 00:36:44 +0800 Message-Id: <20230316163644.42033-1-jamestiotio@gmail.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Cc: James Raphael Tiovalen Subject: [ovs-dev] [PATCH v5] lib, ovsdb, ovs-vsctl, vtep-ctl: Fix multiple Coverity defects X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" This commit addresses several high and medium-impact Coverity defects by fixing several possible null-pointer dereferences and potentially uninitialized variables. There were cases when crashes were encountered when some null pointers were dereferenced. Null pointer checks and alternative code paths have been added to prevent unwanted crashes. Additionally, some struct variables were not initialized. Zero-initializations have been added to prevent potential data leaks or undefined behavior. Some variables would always be initialized by either the code flow or the compiler and some pointers would never be null. Thus, some Coverity reports might be false positives. That said, it is still considered best practice to perform null pointer checks and initialize variables upfront just in case to ensure the overall resilience and security of OVS, as long as they do not impact performance-critical code. As a bonus, it would also make static analyzer tools, such as Coverity, happy. Unit tests have been successfully executed via `make check` and they successfully passed. Signed-off-by: James Raphael Tiovalen --- v5: Improve commit message to better describe the intent of this patch. v4: - Fix some apply-robot checkpatch errors and warnings. - Fix github-robot build failure: linux clang test asan. v3: Fix some apply-robot checkpatch errors and warnings. v2: Fix some apply-robot checkpatch errors and warnings. --- lib/dp-packet.c | 69 ++++++++++++------ lib/dpctl.c | 66 +++++++++-------- lib/json.c | 32 +++++--- lib/latch-unix.c | 2 +- lib/memory.c | 10 ++- lib/netdev-native-tnl.c | 48 +++++++----- lib/odp-execute.c | 91 ++++++++++++----------- lib/pcap-file.c | 5 +- lib/rtnetlink.c | 11 ++- lib/sflow_agent.c | 91 +++++++++++++++-------- lib/shash.c | 4 +- lib/sset.c | 7 +- ovsdb/condition.c | 10 ++- ovsdb/file.c | 26 +++++-- ovsdb/jsonrpc-server.c | 5 +- ovsdb/monitor.c | 49 ++++++++++--- ovsdb/ovsdb-client.c | 45 ++++++++---- ovsdb/ovsdb-server.c | 15 ++-- ovsdb/ovsdb-util.c | 9 +++ ovsdb/ovsdb.c | 124 ++++++++++++++++--------------- ovsdb/query.c | 4 +- ovsdb/replication.c | 14 ++-- ovsdb/row.c | 4 +- ovsdb/table.c | 2 +- ovsdb/transaction.c | 8 +- utilities/ovs-vsctl.c | 157 ++++++++++++++++++++++++---------------- vtep/vtep-ctl.c | 79 +++++++++++--------- 27 files changed, 607 insertions(+), 380 deletions(-) diff --git a/lib/dp-packet.c b/lib/dp-packet.c index ae8ab5800..50cb30681 100644 --- a/lib/dp-packet.c +++ b/lib/dp-packet.c @@ -171,7 +171,11 @@ dp_packet_new_with_headroom(size_t size, size_t headroom) struct dp_packet * dp_packet_clone(const struct dp_packet *buffer) { - return dp_packet_clone_with_headroom(buffer, 0); + if (buffer) { + return dp_packet_clone_with_headroom(buffer, 0); + } else { + return NULL; + } } /* Creates and returns a new dp_packet whose data are copied from 'buffer'. @@ -183,26 +187,32 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom) struct dp_packet *new_buffer; uint32_t mark; - new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), - dp_packet_size(buffer), - headroom); - /* Copy the following fields into the returned buffer: l2_pad_size, - * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */ - memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size, - sizeof(struct dp_packet) - - offsetof(struct dp_packet, l2_pad_size)); + const void *data_dp = dp_packet_data(buffer); - *dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer); - *dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK; + if (data_dp) { + new_buffer = dp_packet_clone_data_with_headroom(data_dp, + dp_packet_size(buffer), + headroom); + /* Copy the following fields into the returned buffer: l2_pad_size, + * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */ + memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size, + sizeof(struct dp_packet) - + offsetof(struct dp_packet, l2_pad_size)); - if (dp_packet_rss_valid(buffer)) { - dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer)); - } - if (dp_packet_has_flow_mark(buffer, &mark)) { - dp_packet_set_flow_mark(new_buffer, mark); - } + *dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer); + *dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK; + + if (dp_packet_rss_valid(buffer)) { + dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer)); + } + if (dp_packet_has_flow_mark(buffer, &mark)) { + dp_packet_set_flow_mark(new_buffer, mark); + } - return new_buffer; + return new_buffer; + } else { + return NULL; + } } /* Creates and returns a new dp_packet that initially contains a copy of the @@ -323,8 +333,11 @@ dp_packet_shift(struct dp_packet *b, int delta) if (delta != 0) { char *dst = (char *) dp_packet_data(b) + delta; - memmove(dst, dp_packet_data(b), dp_packet_size(b)); - dp_packet_set_data(b, dst); + const void *data_dp = dp_packet_data(b); + if (data_dp) { + memmove(dst, data_dp, dp_packet_size(b)); + dp_packet_set_data(b, dst); + } } } @@ -348,7 +361,9 @@ void * dp_packet_put_zeros(struct dp_packet *b, size_t size) { void *dst = dp_packet_put_uninit(b, size); - memset(dst, 0, size); + if (dst) { + memset(dst, 0, size); + } return dst; } @@ -359,7 +374,9 @@ void * dp_packet_put(struct dp_packet *b, const void *p, size_t size) { void *dst = dp_packet_put_uninit(b, size); - memcpy(dst, p, size); + if (dst) { + memcpy(dst, p, size); + } return dst; } @@ -431,7 +448,9 @@ void * dp_packet_push_zeros(struct dp_packet *b, size_t size) { void *dst = dp_packet_push_uninit(b, size); - memset(dst, 0, size); + if (dst) { + memset(dst, 0, size); + } return dst; } @@ -442,7 +461,9 @@ void * dp_packet_push(struct dp_packet *b, const void *p, size_t size) { void *dst = dp_packet_push_uninit(b, size); - memcpy(dst, p, size); + if (dst) { + memcpy(dst, p, size); + } return dst; } diff --git a/lib/dpctl.c b/lib/dpctl.c index c501a0cd7..6472990cc 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -336,12 +336,14 @@ dpctl_add_if(int argc OVS_UNUSED, const char *argv[], value = ""; } - if (!strcmp(key, "type")) { - type = value; - } else if (!strcmp(key, "port_no")) { - port_no = u32_to_odp(atoi(value)); - } else if (!smap_add_once(&args, key, value)) { - dpctl_error(dpctl_p, 0, "duplicate \"%s\" option", key); + if (key) { + if (!strcmp(key, "type")) { + type = value; + } else if (!strcmp(key, "port_no")) { + port_no = u32_to_odp(atoi(value)); + } else if (!smap_add_once(&args, key, value)) { + dpctl_error(dpctl_p, 0, "duplicate \"%s\" option", key); + } } } @@ -454,25 +456,29 @@ dpctl_set_if(int argc, const char *argv[], struct dpctl_params *dpctl_p) value = ""; } - if (!strcmp(key, "type")) { - if (strcmp(value, type)) { - dpctl_error(dpctl_p, 0, - "%s: can't change type from %s to %s", - name, type, value); - error = EINVAL; - goto next_destroy_args; - } - } else if (!strcmp(key, "port_no")) { - if (port_no != u32_to_odp(atoi(value))) { - dpctl_error(dpctl_p, 0, "%s: can't change port number from" - " %"PRIu32" to %d", name, port_no, atoi(value)); - error = EINVAL; - goto next_destroy_args; + if (key) { + if (!strcmp(key, "type")) { + if (strcmp(value, type)) { + dpctl_error(dpctl_p, 0, + "%s: can't change type from %s to %s", + name, type, value); + error = EINVAL; + goto next_destroy_args; + } + } else if (!strcmp(key, "port_no")) { + if (port_no != u32_to_odp(atoi(value))) { + dpctl_error(dpctl_p, 0, + "%s: can't change port number from" + " %"PRIu32" to %d", name, port_no, + atoi(value)); + error = EINVAL; + goto next_destroy_args; + } + } else if (value[0] == '\0') { + smap_remove(&args, key); + } else { + smap_replace(&args, key, value); } - } else if (value[0] == '\0') { - smap_remove(&args, key); - } else { - smap_replace(&args, key, value); } } @@ -693,12 +699,14 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p) error = netdev_get_config(netdev, &config); if (!error) { const struct smap_node **nodes = smap_sort(&config); - for (size_t j = 0; j < smap_count(&config); j++) { - const struct smap_node *node = nodes[j]; - dpctl_print(dpctl_p, "%c %s=%s", j ? ',' : ':', - node->key, node->value); + if (nodes) { + for (size_t j = 0; j < smap_count(&config); j++) { + const struct smap_node *node = nodes[j]; + dpctl_print(dpctl_p, "%c %s=%s", j ? ',' : ':', + node->key, node->value); + } + free(nodes); } - free(nodes); } else { dpctl_print(dpctl_p, ", could not retrieve configuration " "(%s)", ovs_strerror(error)); diff --git a/lib/json.c b/lib/json.c index aded8bb01..09683b54f 100644 --- a/lib/json.c +++ b/lib/json.c @@ -342,8 +342,12 @@ json_object(const struct json *json) bool json_boolean(const struct json *json) { - ovs_assert(json->type == JSON_TRUE || json->type == JSON_FALSE); - return json->type == JSON_TRUE; + if (json) { + ovs_assert(json->type == JSON_TRUE || json->type == JSON_FALSE); + return json->type == JSON_TRUE; + } else { + return false; + } } double @@ -497,13 +501,15 @@ json_hash_object(const struct shash *object, size_t basis) size_t n, i; nodes = shash_sort(object); - n = shash_count(object); - for (i = 0; i < n; i++) { - const struct shash_node *node = nodes[i]; - basis = hash_string(node->name, basis); - basis = json_hash(node->data, basis); + if (nodes) { + n = shash_count(object); + for (i = 0; i < n; i++) { + const struct shash_node *node = nodes[i]; + basis = hash_string(node->name, basis); + basis = json_hash(node->data, basis); + } + free(nodes); } - free(nodes); return basis; } @@ -1654,11 +1660,13 @@ json_serialize_object(const struct shash *object, struct json_serializer *s) size_t n, i; nodes = shash_sort(object); - n = shash_count(object); - for (i = 0; i < n; i++) { - json_serialize_object_member(i, nodes[i], s); + if (nodes) { + n = shash_count(object); + for (i = 0; i < n; i++) { + json_serialize_object_member(i, nodes[i], s); + } + free(nodes); } - free(nodes); } else { struct shash_node *node; size_t i; diff --git a/lib/latch-unix.c b/lib/latch-unix.c index f4d10c39a..262f111e4 100644 --- a/lib/latch-unix.c +++ b/lib/latch-unix.c @@ -71,7 +71,7 @@ latch_set(struct latch *latch) bool latch_is_set(const struct latch *latch) { - struct pollfd pfd; + struct pollfd pfd = {0, 0, 0}; int retval; pfd.fd = latch->fds[0]; diff --git a/lib/memory.c b/lib/memory.c index da97476c6..5ece04807 100644 --- a/lib/memory.c +++ b/lib/memory.c @@ -116,13 +116,15 @@ compose_report(const struct simap *usage, struct ds *s) size_t n = simap_count(usage); size_t i; - for (i = 0; i < n; i++) { - const struct simap_node *node = nodes[i]; + if (nodes) { + for (i = 0; i < n; i++) { + const struct simap_node *node = nodes[i]; - ds_put_format(s, "%s:%u ", node->name, node->data); + ds_put_format(s, "%s:%u ", node->name, node->data); + } + free(nodes); } ds_chomp(s, ' '); - free(nodes); } /* Logs the contents of 'usage', as a collection of name-count pairs. diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index b89dfdd52..e2dd61880 100644 --- a/lib/netdev-native-tnl.c +++ b/lib/netdev-native-tnl.c @@ -221,16 +221,20 @@ netdev_tnl_calc_udp_csum(struct udp_header *udp, struct dp_packet *packet, { uint32_t csum; - if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) { - csum = packet_csum_pseudoheader6(netdev_tnl_ipv6_hdr( - dp_packet_data(packet))); - } else { - csum = packet_csum_pseudoheader(netdev_tnl_ip_hdr( - dp_packet_data(packet))); - } + void *data_dp = dp_packet_data(packet); + + if (data_dp) { + if (netdev_tnl_is_header_ipv6(data_dp)) { + csum = packet_csum_pseudoheader6(netdev_tnl_ipv6_hdr( + data_dp)); + } else { + csum = packet_csum_pseudoheader(netdev_tnl_ip_hdr( + data_dp)); + } - csum = csum_continue(csum, udp, ip_tot_size); - udp->udp_csum = csum_finish(csum); + csum = csum_continue(csum, udp, ip_tot_size); + udp->udp_csum = csum_finish(csum); + } if (!udp->udp_csum) { udp->udp_csum = htons(0xffff); @@ -425,20 +429,24 @@ netdev_gre_pop_header(struct dp_packet *packet) struct flow_tnl *tnl = &md->tunnel; int hlen = sizeof(struct eth_header) + 4; - hlen += netdev_tnl_is_header_ipv6(dp_packet_data(packet)) ? - IPV6_HEADER_LEN : IP_HEADER_LEN; + const void *data_dp = dp_packet_data(packet); - pkt_metadata_init_tnl(md); - if (hlen > dp_packet_size(packet)) { - goto err; - } + if (data_dp) { + hlen += netdev_tnl_is_header_ipv6(data_dp) ? + IPV6_HEADER_LEN : IP_HEADER_LEN; - hlen = parse_gre_header(packet, tnl); - if (hlen < 0) { - goto err; - } + pkt_metadata_init_tnl(md); + if (hlen > dp_packet_size(packet)) { + goto err; + } - dp_packet_reset_packet(packet, hlen); + hlen = parse_gre_header(packet, tnl); + if (hlen < 0) { + goto err; + } + + dp_packet_reset_packet(packet, hlen); + } return packet; err: diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 5cf6fbec0..b8cf3db55 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -147,42 +147,45 @@ odp_set_ipv4(struct dp_packet *packet, const struct ovs_key_ipv4 *key, uint8_t new_tos; uint8_t new_ttl; - if (mask->ipv4_src) { - ip_src_nh = get_16aligned_be32(&nh->ip_src); - new_ip_src = key->ipv4_src | (ip_src_nh & ~mask->ipv4_src); + if (nh) { + if (mask->ipv4_src) { + ip_src_nh = get_16aligned_be32(&nh->ip_src); + new_ip_src = key->ipv4_src | (ip_src_nh & ~mask->ipv4_src); - if (ip_src_nh != new_ip_src) { - packet_set_ipv4_addr(packet, &nh->ip_src, new_ip_src); + if (ip_src_nh != new_ip_src) { + packet_set_ipv4_addr(packet, &nh->ip_src, new_ip_src); + } } - } - if (mask->ipv4_dst) { - ip_dst_nh = get_16aligned_be32(&nh->ip_dst); - new_ip_dst = key->ipv4_dst | (ip_dst_nh & ~mask->ipv4_dst); + if (mask->ipv4_dst) { + ip_dst_nh = get_16aligned_be32(&nh->ip_dst); + new_ip_dst = key->ipv4_dst | (ip_dst_nh & ~mask->ipv4_dst); - if (ip_dst_nh != new_ip_dst) { - packet_set_ipv4_addr(packet, &nh->ip_dst, new_ip_dst); + if (ip_dst_nh != new_ip_dst) { + packet_set_ipv4_addr(packet, &nh->ip_dst, new_ip_dst); + } } - } - if (mask->ipv4_tos) { - new_tos = key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos); + if (mask->ipv4_tos) { + new_tos = key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos); - if (nh->ip_tos != new_tos) { - nh->ip_csum = recalc_csum16(nh->ip_csum, - htons((uint16_t) nh->ip_tos), - htons((uint16_t) new_tos)); - nh->ip_tos = new_tos; + if (nh->ip_tos != new_tos) { + nh->ip_csum = recalc_csum16(nh->ip_csum, + htons((uint16_t) nh->ip_tos), + htons((uint16_t) new_tos)); + nh->ip_tos = new_tos; + } } - } - if (OVS_LIKELY(mask->ipv4_ttl)) { - new_ttl = key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl); + if (OVS_LIKELY(mask->ipv4_ttl)) { + new_ttl = key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl); - if (OVS_LIKELY(nh->ip_ttl != new_ttl)) { - nh->ip_csum = recalc_csum16(nh->ip_csum, htons(nh->ip_ttl << 8), - htons(new_ttl << 8)); - nh->ip_ttl = new_ttl; + if (OVS_LIKELY(nh->ip_ttl != new_ttl)) { + nh->ip_csum = recalc_csum16(nh->ip_csum, + htons(nh->ip_ttl << 8), + htons(new_ttl << 8)); + nh->ip_ttl = new_ttl; + } } } } @@ -276,23 +279,25 @@ set_arp(struct dp_packet *packet, const struct ovs_key_arp *key, { struct arp_eth_header *arp = dp_packet_l3(packet); - if (!mask) { - arp->ar_op = key->arp_op; - arp->ar_sha = key->arp_sha; - put_16aligned_be32(&arp->ar_spa, key->arp_sip); - arp->ar_tha = key->arp_tha; - put_16aligned_be32(&arp->ar_tpa, key->arp_tip); - } else { - ovs_be32 ar_spa = get_16aligned_be32(&arp->ar_spa); - ovs_be32 ar_tpa = get_16aligned_be32(&arp->ar_tpa); - - arp->ar_op = key->arp_op | (arp->ar_op & ~mask->arp_op); - ether_addr_copy_masked(&arp->ar_sha, key->arp_sha, mask->arp_sha); - put_16aligned_be32(&arp->ar_spa, - key->arp_sip | (ar_spa & ~mask->arp_sip)); - ether_addr_copy_masked(&arp->ar_tha, key->arp_tha, mask->arp_tha); - put_16aligned_be32(&arp->ar_tpa, - key->arp_tip | (ar_tpa & ~mask->arp_tip)); + if (arp) { + if (!mask) { + arp->ar_op = key->arp_op; + arp->ar_sha = key->arp_sha; + put_16aligned_be32(&arp->ar_spa, key->arp_sip); + arp->ar_tha = key->arp_tha; + put_16aligned_be32(&arp->ar_tpa, key->arp_tip); + } else { + ovs_be32 ar_spa = get_16aligned_be32(&arp->ar_spa); + ovs_be32 ar_tpa = get_16aligned_be32(&arp->ar_tpa); + + arp->ar_op = key->arp_op | (arp->ar_op & ~mask->arp_op); + ether_addr_copy_masked(&arp->ar_sha, key->arp_sha, mask->arp_sha); + put_16aligned_be32(&arp->ar_spa, + key->arp_sip | (ar_spa & ~mask->arp_sip)); + ether_addr_copy_masked(&arp->ar_tha, key->arp_tha, mask->arp_tha); + put_16aligned_be32(&arp->ar_tpa, + key->arp_tip | (ar_tpa & ~mask->arp_tip)); + } } } diff --git a/lib/pcap-file.c b/lib/pcap-file.c index 3ed7ea488..7cb6f5b2a 100644 --- a/lib/pcap-file.c +++ b/lib/pcap-file.c @@ -291,7 +291,10 @@ ovs_pcap_write(struct pcap_file *p_file, struct dp_packet *buf) prh.incl_len = dp_packet_size(buf); prh.orig_len = dp_packet_size(buf); ignore(fwrite(&prh, sizeof prh, 1, p_file->file)); - ignore(fwrite(dp_packet_data(buf), dp_packet_size(buf), 1, p_file->file)); + const void *data_dp = dp_packet_data(buf); + if (data_dp) { + ignore(fwrite(data_dp, dp_packet_size(buf), 1, p_file->file)); + } fflush(p_file->file); } diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c index f67352603..67a0134ed 100644 --- a/lib/rtnetlink.c +++ b/lib/rtnetlink.c @@ -131,10 +131,13 @@ rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change *change) change->irrelevant = true; } + if (ifinfo) { + change->if_index = ifinfo->ifi_index; + change->ifi_flags = ifinfo->ifi_flags; + } + change->nlmsg_type = nlmsg->nlmsg_type; - change->if_index = ifinfo->ifi_index; change->ifname = nl_attr_get_string(attrs[IFLA_IFNAME]); - change->ifi_flags = ifinfo->ifi_flags; change->master_ifindex = (attrs[IFLA_MASTER] ? nl_attr_get_u32(attrs[IFLA_MASTER]) : 0); @@ -178,7 +181,9 @@ rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change *change) ifaddr = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifaddr); change->nlmsg_type = nlmsg->nlmsg_type; - change->if_index = ifaddr->ifa_index; + if (ifaddr) { + change->if_index = ifaddr->ifa_index; + } change->ifname = (attrs[IFA_LABEL] ? nl_attr_get_string(attrs[IFA_LABEL]) : NULL); diff --git a/lib/sflow_agent.c b/lib/sflow_agent.c index c95f654a5..c19434d6c 100644 --- a/lib/sflow_agent.c +++ b/lib/sflow_agent.c @@ -152,15 +152,24 @@ void sfl_agent_tick(SFLAgent *agent, time_t now) SFLReceiver *sfl_agent_addReceiver(SFLAgent *agent) { - SFLReceiver *rcv = (SFLReceiver *)sflAlloc(agent, sizeof(SFLReceiver)); - sfl_receiver_init(rcv, agent); - /* add to end of list - to preserve the receiver index numbers for existing receivers */ - { - SFLReceiver *r, *prev = NULL; - for(r = agent->receivers; r != NULL; prev = r, r = r->nxt); - if(prev) prev->nxt = rcv; - else agent->receivers = rcv; - rcv->nxt = NULL; + SFLReceiver *rcv = (SFLReceiver *) sflAlloc(agent, sizeof(SFLReceiver)); + if (rcv) { + sfl_receiver_init(rcv, agent); + /* add to end of list - to preserve the receiver index numbers for + * existing receivers + */ + { + SFLReceiver *r, *prev = NULL; + for (r = agent->receivers; r != NULL; prev = r, r = r->nxt) { + /* nothing to do */ + } + if (prev) { + prev->nxt = rcv; + } else { + agent->receivers = rcv; + } + rcv->nxt = NULL; + } } return rcv; } @@ -202,23 +211,35 @@ SFLSampler *sfl_agent_addSampler(SFLAgent *agent, SFLDataSource_instance *pdsi) /* either we found the insert point, or reached the end of the list...*/ { - SFLSampler *newsm = (SFLSampler *)sflAlloc(agent, sizeof(SFLSampler)); - sfl_sampler_init(newsm, agent, pdsi); - if(prev) prev->nxt = newsm; - else agent->samplers = newsm; - newsm->nxt = sm; - - /* see if we should go in the ifIndex jumpTable */ - if(SFL_DS_CLASS(newsm->dsi) == 0) { - SFLSampler *test = sfl_agent_getSamplerByIfIndex(agent, SFL_DS_INDEX(newsm->dsi)); - if(test && (SFL_DS_INSTANCE(newsm->dsi) < SFL_DS_INSTANCE(test->dsi))) { - /* replace with this new one because it has a lower ds_instance number */ - sfl_agent_jumpTableRemove(agent, test); - test = NULL; - } - if(test == NULL) sfl_agent_jumpTableAdd(agent, newsm); - } - return newsm; + SFLSampler *newsm = (SFLSampler *) sflAlloc(agent, sizeof(SFLSampler)); + if (newsm) { + memset(newsm, 0, sizeof(*newsm)); + sfl_sampler_init(newsm, agent, pdsi); + if (prev) { + prev->nxt = newsm; + } else { + agent->samplers = newsm; + } + newsm->nxt = sm; + + /* see if we should go in the ifIndex jumpTable */ + if (SFL_DS_CLASS(newsm->dsi) == 0) { + SFLSampler *test = sfl_agent_getSamplerByIfIndex(agent, + SFL_DS_INDEX(newsm->dsi)); + if (test && (SFL_DS_INSTANCE(newsm->dsi) + < SFL_DS_INSTANCE(test->dsi))) { + /* replace with this new one because it has a lower + * ds_instance number + */ + sfl_agent_jumpTableRemove(agent, test); + test = NULL; + } + if (test == NULL) { + sfl_agent_jumpTableAdd(agent, newsm); + } + } + } + return newsm; } } @@ -241,12 +262,18 @@ SFLPoller *sfl_agent_addPoller(SFLAgent *agent, } /* either we found the insert point, or reached the end of the list... */ { - SFLPoller *newpl = (SFLPoller *)sflAlloc(agent, sizeof(SFLPoller)); - sfl_poller_init(newpl, agent, pdsi, magic, getCountersFn); - if(prev) prev->nxt = newpl; - else agent->pollers = newpl; - newpl->nxt = pl; - return newpl; + SFLPoller *newpl = (SFLPoller *) sflAlloc(agent, sizeof(SFLPoller)); + if (newpl) { + memset(newpl, 0, sizeof(*newpl)); + sfl_poller_init(newpl, agent, pdsi, magic, getCountersFn); + if (prev) { + prev->nxt = newpl; + } else { + agent->pollers = newpl; + } + newpl->nxt = pl; + } + return newpl; } } diff --git a/lib/shash.c b/lib/shash.c index a7b2c6458..adb388cf7 100644 --- a/lib/shash.c +++ b/lib/shash.c @@ -194,7 +194,9 @@ shash_replace_nocopy(struct shash *sh, char *name, const void *data) void shash_delete(struct shash *sh, struct shash_node *node) { - free(shash_steal(sh, node)); + if (node) { + free(shash_steal(sh, node)); + } } /* Deletes 'node' from 'sh'. Neither the node's name nor its data is freed; diff --git a/lib/sset.c b/lib/sset.c index aa1790020..50f6676df 100644 --- a/lib/sset.c +++ b/lib/sset.c @@ -261,8 +261,11 @@ char * sset_pop(struct sset *set) { const char *name = SSET_FIRST(set); - char *copy = xstrdup(name); - sset_delete(set, SSET_NODE_FROM_NAME(name)); + char *copy = NULL; + if (name) { + copy = xstrdup(name); + sset_delete(set, SSET_NODE_FROM_NAME(name)); + } return copy; } diff --git a/ovsdb/condition.c b/ovsdb/condition.c index d0016fa7f..f36531669 100644 --- a/ovsdb/condition.c +++ b/ovsdb/condition.c @@ -47,7 +47,15 @@ ovsdb_clause_from_json(const struct ovsdb_table_schema *ts, /* Column and arg fields are not being used with boolean functions. * Use dummy values */ - clause->column = ovsdb_table_schema_get_column(ts, "_uuid"); + const struct ovsdb_column *uuid_column; + uuid_column = ovsdb_table_schema_get_column(ts, "_uuid"); + if (uuid_column) { + clause->column = uuid_column; + } else { + return ovsdb_syntax_error(json, + "unknown column", + "No column _uuid in table schema."); + } clause->index = clause->column->index; ovsdb_datum_init_default(&clause->arg, &clause->column->type); return NULL; diff --git a/ovsdb/file.c b/ovsdb/file.c index 2d887e53e..bccb76098 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -522,7 +522,12 @@ ovsdb_file_txn_add_row(struct ovsdb_file_txn *ftxn, } if (row) { - struct ovsdb_table *table = new ? new->table : old->table; + struct ovsdb_table *table = NULL; + if (new) { + table = new->table; + } else if (old) { + table = old->table; + } char uuid[UUID_LEN + 1]; if (table != ftxn->table) { @@ -533,14 +538,23 @@ ovsdb_file_txn_add_row(struct ovsdb_file_txn *ftxn, /* Create JSON object for transaction on this table. */ ftxn->table_json = json_object_create(); - ftxn->table = table; - json_object_put(ftxn->json, table->schema->name, ftxn->table_json); + if (table) { + ftxn->table = table; + json_object_put(ftxn->json, table->schema->name, + ftxn->table_json); + } } /* Add row to transaction for this table. */ - snprintf(uuid, sizeof uuid, - UUID_FMT, UUID_ARGS(ovsdb_row_get_uuid(new ? new : old))); - json_object_put(ftxn->table_json, uuid, row); + if (new) { + snprintf(uuid, sizeof uuid, + UUID_FMT, UUID_ARGS(ovsdb_row_get_uuid(new))); + json_object_put(ftxn->table_json, uuid, row); + } else if (old) { + snprintf(uuid, sizeof uuid, + UUID_FMT, UUID_ARGS(ovsdb_row_get_uuid(old))); + json_object_put(ftxn->table_json, uuid, row); + } } } diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c index 17868f5b7..b1cbaa227 100644 --- a/ovsdb/jsonrpc-server.c +++ b/ovsdb/jsonrpc-server.c @@ -1038,7 +1038,7 @@ ovsdb_jsonrpc_session_got_request(struct ovsdb_jsonrpc_session *s, request->id); } else if (!strcmp(request->method, "get_schema")) { struct ovsdb *db = ovsdb_jsonrpc_lookup_db(s, request, &reply); - if (!reply) { + if (db && !reply) { reply = jsonrpc_create_reply(ovsdb_schema_to_json(db->schema), request->id); } @@ -1131,6 +1131,8 @@ static void ovsdb_jsonrpc_trigger_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db, struct jsonrpc_msg *request) { + ovs_assert(db); + /* Check for duplicate ID. */ size_t hash = json_hash(request->id, 0); struct ovsdb_jsonrpc_trigger *t @@ -1391,6 +1393,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db, enum ovsdb_monitor_version version, const struct json *request_id) { + ovs_assert(db); struct ovsdb_jsonrpc_monitor *m = NULL; struct ovsdb_monitor *dbmon = NULL; struct json *monitor_id, *monitor_requests; diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c index 191befcae..1ea92cc0f 100644 --- a/ovsdb/monitor.c +++ b/ovsdb/monitor.c @@ -478,7 +478,9 @@ ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon, struct ovsdb_monitor_column *c; mt = shash_find_data(&dbmon->tables, table->schema->name); - + if (!mt) { + return NULL; + } /* Check for column duplication. Return duplicated column name. */ if (mt->columns_index_map[column->index] != -1) { return column->name; @@ -781,11 +783,15 @@ ovsdb_monitor_table_condition_update( const struct json *cond_json) { if (!condition) { - return NULL; + return ovsdb_syntax_error(cond_json, NULL, + "Parse error, condition empty."); } struct ovsdb_monitor_table_condition *mtc = shash_find_data(&condition->tables, table->schema->name); + if (!mtc) { + return NULL; + } struct ovsdb_error *error; struct ovsdb_condition cond = OVSDB_CONDITION_INITIALIZER(&cond); @@ -1279,7 +1285,9 @@ ovsdb_monitor_table_add_select(struct ovsdb_monitor *dbmon, struct ovsdb_monitor_table * mt; mt = shash_find_data(&dbmon->tables, table->schema->name); - mt->select |= select; + if (mt) { + mt->select |= select; + } } /* @@ -1329,8 +1337,23 @@ ovsdb_monitor_changes_update(const struct ovsdb_row *old, const struct ovsdb_monitor_table *mt, struct ovsdb_monitor_change_set_for_table *mcst) { - const struct uuid *uuid = ovsdb_row_get_uuid(new ? new : old); - struct ovsdb_monitor_row *change; + const struct uuid *uuid = NULL; + + if (!new && !old) { + return; + } else { + if (new) { + uuid = ovsdb_row_get_uuid(new); + } else if (old) { + uuid = ovsdb_row_get_uuid(old); + } + } + + if (!uuid) { + return; + } + + struct ovsdb_monitor_row *change = NULL; change = ovsdb_monitor_changes_row_find(mcst, uuid); if (!change) { @@ -1657,15 +1680,21 @@ ovsdb_monitor_hash(const struct ovsdb_monitor *dbmon, size_t basis) nodes = shash_sort(&dbmon->tables); n = shash_count(&dbmon->tables); + if (!nodes) { + return basis; + } + for (i = 0; i < n; i++) { struct ovsdb_monitor_table *mt = nodes[i]->data; - basis = hash_pointer(mt->table, basis); - basis = hash_3words(mt->select, mt->n_columns, basis); + if (mt) { + basis = hash_pointer(mt->table, basis); + basis = hash_3words(mt->select, mt->n_columns, basis); - for (j = 0; j < mt->n_columns; j++) { - basis = hash_pointer(mt->columns[j].column, basis); - basis = hash_2words(mt->columns[j].select, basis); + for (j = 0; j < mt->n_columns; j++) { + basis = hash_pointer(mt->columns[j].column, basis); + basis = hash_2words(mt->columns[j].select, basis); + } } } free(nodes); diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c index f1b8d6491..ca2becd8d 100644 --- a/ovsdb/ovsdb-client.c +++ b/ovsdb/ovsdb-client.c @@ -1223,17 +1223,25 @@ parse_monitor_columns(char *arg, const char *server, const char *database, n = shash_count(&table->columns); nodes = shash_sort(&table->columns); - for (i = 0; i < n; i++) { - const struct ovsdb_column *column = nodes[i]->data; - if (column->index != OVSDB_COL_UUID - && column->index != OVSDB_COL_VERSION) { - add_column(server, column, columns, columns_json); + if (nodes) { + for (i = 0; i < n; i++) { + const struct ovsdb_column *column = nodes[i]->data; + if (column->index != OVSDB_COL_UUID + && column->index != OVSDB_COL_VERSION) { + add_column(server, column, columns, columns_json); + } } + free(nodes); } - free(nodes); - add_column(server, ovsdb_table_schema_get_column(table, "_version"), - columns, columns_json); + const struct ovsdb_column *version_column = + ovsdb_table_schema_get_column(table, "_version"); + + if (version_column) { + add_column(server, version_column, columns, columns_json); + } else { + VLOG_ERR("Table does not contain _version column."); + } } if (!initial || !insert || !delete || !modify) { @@ -1439,14 +1447,16 @@ do_monitor__(struct jsonrpc *rpc, const char *database, ovs_fatal(0, "ALL tables are not allowed with condition"); } - for (i = 0; i < n; i++) { - struct ovsdb_table_schema *table = nodes[i]->data; + if (nodes) { + for (i = 0; i < n; i++) { + struct ovsdb_table_schema *table = nodes[i]->data; - add_monitored_table(argc, argv, server, database, NULL, table, - monitor_requests, - &mts, &n_mts, &allocated_mts); + add_monitored_table(argc, argv, server, database, NULL, table, + monitor_requests, + &mts, &n_mts, &allocated_mts); + } + free(nodes); } - free(nodes); } send_db_change_aware(rpc); @@ -1870,6 +1880,10 @@ do_dump(struct jsonrpc *rpc, const char *database, n_tables = shash_count(&schema->tables); } + if (!tables) { + goto end; + } + /* Construct transaction to retrieve entire database. */ transaction = json_array_create_1(json_string_create(database)); for (i = 0; i < n_tables; i++) { @@ -1929,8 +1943,9 @@ do_dump(struct jsonrpc *rpc, const char *database, } jsonrpc_msg_destroy(reply); - shash_destroy(&custom_columns); free(tables); +end: + shash_destroy(&custom_columns); ovsdb_schema_destroy(schema); } diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 33ca4910d..02e024de4 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -1784,14 +1784,16 @@ ovsdb_server_list_databases(struct unixctl_conn *conn, int argc OVS_UNUSED, ds_init(&s); nodes = shash_sort(all_dbs); - for (i = 0; i < shash_count(all_dbs); i++) { - const struct shash_node *node = nodes[i]; - struct db *db = node->data; - if (db->db) { - ds_put_format(&s, "%s\n", node->name); + if (nodes) { + for (i = 0; i < shash_count(all_dbs); i++) { + const struct shash_node *node = nodes[i]; + struct db *db = node->data; + if (db->db) { + ds_put_format(&s, "%s\n", node->name); + } } + free(nodes); } - free(nodes); unixctl_command_reply(conn, ds_cstr(&s)); ds_destroy(&s); @@ -2191,6 +2193,7 @@ save_config(struct server_config *config) static void sset_from_json(struct sset *sset, const struct json *array) { + ovs_assert(array); size_t i; sset_clear(sset); diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c index 303191dc8..148230da8 100644 --- a/ovsdb/ovsdb-util.c +++ b/ovsdb/ovsdb-util.c @@ -291,6 +291,15 @@ ovsdb_util_write_string_string_column(struct ovsdb_row *row, size_t i; column = ovsdb_table_schema_get_column(row->table->schema, column_name); + if (!column) { + VLOG_ERR("No %s column present in the %s table", + column_name, row->table->schema->name); + for (i = 0; i < n; i++) { + free(keys[i]); + free(values[i]); + } + return; + } datum = ovsdb_util_get_datum(row, column_name, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING, UINT_MAX); if (!datum) { diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c index afec96264..6dcfc5751 100644 --- a/ovsdb/ovsdb.c +++ b/ovsdb/ovsdb.c @@ -195,7 +195,7 @@ root_set_size(const struct ovsdb_schema *schema) struct ovsdb_error * ovsdb_schema_from_json(const struct json *json, struct ovsdb_schema **schemap) { - struct ovsdb_schema *schema; + struct ovsdb_schema *schema = NULL; const struct json *name, *tables, *version_json, *cksum; struct ovsdb_error *error; struct shash_node *node; @@ -215,78 +215,86 @@ ovsdb_schema_from_json(const struct json *json, struct ovsdb_schema **schemap) return error; } - if (version_json) { - version = json_string(version_json); - if (!ovsdb_is_valid_version(version)) { - return ovsdb_syntax_error(json, NULL, "schema version \"%s\" not " - "in format x.y.z", version); + if (name && tables) { + if (version_json) { + version = json_string(version_json); + if (!ovsdb_is_valid_version(version)) { + return ovsdb_syntax_error(json, NULL, + "schema version \"%s\" not " + "in format x.y.z", version); + } + } else { + /* Backward compatibility with old databases. */ + version = ""; } - } else { - /* Backward compatibility with old databases. */ - version = ""; - } - schema = ovsdb_schema_create(json_string(name), version, - cksum ? json_string(cksum) : ""); - SHASH_FOR_EACH (node, json_object(tables)) { - struct ovsdb_table_schema *table; + schema = ovsdb_schema_create(json_string(name), version, + cksum ? json_string(cksum) : ""); + SHASH_FOR_EACH (node, json_object(tables)) { + struct ovsdb_table_schema *table; + + if (node->name[0] == '_') { + error = ovsdb_syntax_error(json, NULL, "names beginning with " + "\"_\" are reserved"); + } else if (!ovsdb_parser_is_id(node->name)) { + error = ovsdb_syntax_error(json, NULL, + "name must be a valid id"); + } else { + error = ovsdb_table_schema_from_json(node->data, node->name, + &table); + } + if (error) { + ovsdb_schema_destroy(schema); + return error; + } - if (node->name[0] == '_') { - error = ovsdb_syntax_error(json, NULL, "names beginning with " - "\"_\" are reserved"); - } else if (!ovsdb_parser_is_id(node->name)) { - error = ovsdb_syntax_error(json, NULL, "name must be a valid id"); - } else { - error = ovsdb_table_schema_from_json(node->data, node->name, - &table); - } - if (error) { - ovsdb_schema_destroy(schema); - return error; + shash_add(&schema->tables, table->name, table); } - - shash_add(&schema->tables, table->name, table); } - /* "isRoot" was not part of the original schema definition. Before it was - * added, there was no support for garbage collection. So, for backward - * compatibility, if the root set is empty then assume that every table is - * in the root set. */ - if (root_set_size(schema) == 0) { - SHASH_FOR_EACH (node, &schema->tables) { - struct ovsdb_table_schema *table = node->data; - - table->is_root = true; + if (schema) { + /* "isRoot" was not part of the original schema definition. Before it + * was added, there was no support for garbage collection. So, for + * backward compatibility, if the root set is empty then assume that + * every table is in the root set. */ + if (root_set_size(schema) == 0) { + SHASH_FOR_EACH (node, &schema->tables) { + struct ovsdb_table_schema *table = node->data; + + table->is_root = true; + } } - } - /* Validate that all refTables refer to the names of tables that exist. - * - * Also force certain columns to be persistent, as explained in - * ovsdb_schema_check_ref_table(). This requires 'is_root' to be known, so - * this must follow the loop updating 'is_root' above. */ - SHASH_FOR_EACH (node, &schema->tables) { - struct ovsdb_table_schema *table = node->data; - struct shash_node *node2; + /* Validate that all refTables refer to the names of tables that exist. + * + * Also force certain columns to be persistent, as explained in + * ovsdb_schema_check_ref_table(). This requires 'is_root' to be + * known, so this must follow the loop updating 'is_root' above. */ + SHASH_FOR_EACH (node, &schema->tables) { + struct ovsdb_table_schema *table = node->data; + struct shash_node *node2; - SHASH_FOR_EACH (node2, &table->columns) { - struct ovsdb_column *column = node2->data; + SHASH_FOR_EACH (node2, &table->columns) { + struct ovsdb_column *column = node2->data; - error = ovsdb_schema_check_ref_table(column, &schema->tables, - &column->type.key, "key"); - if (!error) { error = ovsdb_schema_check_ref_table(column, &schema->tables, - &column->type.value, - "value"); - } - if (error) { - ovsdb_schema_destroy(schema); - return error; + &column->type.key, "key"); + if (!error) { + error = ovsdb_schema_check_ref_table(column, + &schema->tables, + &column->type.value, + "value"); + } + if (error) { + ovsdb_schema_destroy(schema); + return error; + } } } + + *schemap = schema; } - *schemap = schema; return NULL; } diff --git a/ovsdb/query.c b/ovsdb/query.c index eebe56412..0129eb038 100644 --- a/ovsdb/query.c +++ b/ovsdb/query.c @@ -91,7 +91,9 @@ ovsdb_query_distinct(struct ovsdb_table *table, struct ovsdb_row_hash hash; ovsdb_row_hash_init(&hash, columns); - ovsdb_query(table, condition, query_distinct_cb, &hash); + if (condition) { + ovsdb_query(table, condition, query_distinct_cb, &hash); + } HMAP_FOR_EACH (node, hmap_node, &hash.rows) { ovsdb_row_set_add_row(results, node->row); } diff --git a/ovsdb/replication.c b/ovsdb/replication.c index 477c69d70..f5e150c33 100644 --- a/ovsdb/replication.c +++ b/ovsdb/replication.c @@ -575,15 +575,17 @@ create_monitor_request(struct ovsdb_schema *schema) size_t n = shash_count(&schema->tables); const struct shash_node **nodes = shash_sort(&schema->tables); - for (int j = 0; j < n; j++) { - struct ovsdb_table_schema *table = nodes[j]->data; + if (nodes) { + for (int j = 0; j < n; j++) { + struct ovsdb_table_schema *table = nodes[j]->data; - /* Monitor all tables not excluded. */ - if (!excluded_tables_find(db_name, table->name)) { - add_monitored_table(table, monitor_request); + /* Monitor all tables not excluded. */ + if (!excluded_tables_find(db_name, table->name)) { + add_monitored_table(table, monitor_request); + } } + free(nodes); } - free(nodes); /* Create a monitor request. */ monitor = json_array_create_3( diff --git a/ovsdb/row.c b/ovsdb/row.c index d7bfbdd36..9f87c832a 100644 --- a/ovsdb/row.c +++ b/ovsdb/row.c @@ -399,7 +399,9 @@ ovsdb_row_set_add_row(struct ovsdb_row_set *set, const struct ovsdb_row *row) set->rows = x2nrealloc(set->rows, &set->allocated_rows, sizeof *set->rows); } - set->rows[set->n_rows++] = row; + if (set->rows) { + set->rows[set->n_rows++] = row; + } } struct json * diff --git a/ovsdb/table.c b/ovsdb/table.c index 66071ce2f..990d49ea4 100644 --- a/ovsdb/table.c +++ b/ovsdb/table.c @@ -158,7 +158,7 @@ ovsdb_table_schema_from_json(const struct json *json, const char *name, n_max_rows = UINT_MAX; } - if (shash_is_empty(json_object(columns))) { + if (!columns || shash_is_empty(json_object(columns))) { return ovsdb_syntax_error(json, NULL, "table must have at least one column"); } diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index 03541af85..43b104f31 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -576,9 +576,11 @@ ovsdb_txn_update_weak_refs(struct ovsdb_txn *txn OVS_UNUSED, dst_row = CONST_CAST(struct ovsdb_row *, ovsdb_table_get_row(weak->dst_table, &weak->dst)); - ovs_assert(!ovsdb_row_find_weak_ref(dst_row, weak)); - hmap_insert(&dst_row->dst_refs, &weak->dst_node, - ovsdb_weak_ref_hash(weak)); + if (dst_row) { + ovs_assert(!ovsdb_row_find_weak_ref(dst_row, weak)); + hmap_insert(&dst_row->dst_refs, &weak->dst_node, + ovsdb_weak_ref_hash(weak)); + } ovs_list_remove(&weak->src_node); ovs_list_init(&weak->src_node); } diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 2f5ac1a26..991b426b9 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -334,8 +334,11 @@ parse_options(int argc, char *argv[], struct shash *local_options) exit(EXIT_SUCCESS); case 't': - if (!str_to_uint(optarg, 10, &timeout) || !timeout) { - ctl_fatal("value %s on -t or --timeout is invalid", optarg); + if (optarg) { + if (!str_to_uint(optarg, 10, &timeout) || !timeout) { + ctl_fatal("value %s on -t or --timeout is invalid", + optarg); + } } break; @@ -349,11 +352,15 @@ parse_options(int argc, char *argv[], struct shash *local_options) STREAM_SSL_OPTION_HANDLERS case OPT_PEER_CA_CERT: - stream_ssl_set_peer_ca_cert_file(optarg); + if (optarg) { + stream_ssl_set_peer_ca_cert_file(optarg); + } break; case OPT_BOOTSTRAP_CA_CERT: - stream_ssl_set_ca_cert_file(optarg, true); + if (optarg) { + stream_ssl_set_ca_cert_file(optarg, true); + } break; case '?': @@ -575,7 +582,7 @@ add_bridge_to_cache(struct vsctl_context *vsctl_ctx, struct ovsrec_bridge *br_cfg, const char *name, struct vsctl_bridge *parent, int vlan) { - struct vsctl_bridge *br = xmalloc(sizeof *br); + struct vsctl_bridge *br = xzalloc(sizeof *br); br->br_cfg = br_cfg; br->name = xstrdup(name); ovs_list_init(&br->ports); @@ -659,7 +666,7 @@ static struct vsctl_port * add_port_to_cache(struct vsctl_context *vsctl_ctx, struct vsctl_bridge *parent, struct ovsrec_port *port_cfg) { - struct vsctl_port *port; + struct vsctl_port *port = xzalloc(sizeof *port); if (port_cfg->tag && *port_cfg->tag >= 0 && *port_cfg->tag <= 4095) { @@ -671,11 +678,13 @@ add_port_to_cache(struct vsctl_context *vsctl_ctx, struct vsctl_bridge *parent, } } - port = xmalloc(sizeof *port); - ovs_list_push_back(&parent->ports, &port->ports_node); + if (parent) { + ovs_list_push_back(&parent->ports, &port->ports_node); + port->bridge = parent; + } + ovs_list_init(&port->ifaces); port->port_cfg = port_cfg; - port->bridge = parent; shash_add(&vsctl_ctx->ports, port_cfg->name, port); return port; @@ -818,55 +827,63 @@ vsctl_context_populate_cache(struct ctl_context *ctx) continue; } br = shash_find_data(&vsctl_ctx->bridges, br_cfg->name); - for (j = 0; j < br_cfg->n_ports; j++) { - struct ovsrec_port *port_cfg = br_cfg->ports[j]; - struct vsctl_port *port; - size_t k; - - port = shash_find_data(&vsctl_ctx->ports, port_cfg->name); - if (port) { - if (port_cfg == port->port_cfg) { - VLOG_WARN("%s: port is in multiple bridges (%s and %s)", - port_cfg->name, br->name, port->bridge->name); - } else { - /* Log as an error because this violates the database's - * uniqueness constraints, so the database server shouldn't - * have allowed it. */ - VLOG_ERR("%s: database contains duplicate port name", - port_cfg->name); - } - continue; - } - - if (port_is_fake_bridge(port_cfg) - && !sset_add(&bridges, port_cfg->name)) { - continue; - } - - port = add_port_to_cache(vsctl_ctx, br, port_cfg); - for (k = 0; k < port_cfg->n_interfaces; k++) { - struct ovsrec_interface *iface_cfg = port_cfg->interfaces[k]; - struct vsctl_iface *iface; - - iface = shash_find_data(&vsctl_ctx->ifaces, iface_cfg->name); - if (iface) { - if (iface_cfg == iface->iface_cfg) { - VLOG_WARN("%s: interface is in multiple ports " - "(%s and %s)", - iface_cfg->name, - iface->port->port_cfg->name, - port->port_cfg->name); + if (br) { + for (j = 0; j < br_cfg->n_ports; j++) { + struct ovsrec_port *port_cfg = br_cfg->ports[j]; + struct vsctl_port *port; + size_t k; + + port = shash_find_data(&vsctl_ctx->ports, port_cfg->name); + if (port) { + if (port_cfg == port->port_cfg) { + VLOG_WARN("%s: port is in multiple bridges " + "(%s and %s)", port_cfg->name, br->name, + port->bridge->name); } else { /* Log as an error because this violates the database's - * uniqueness constraints, so the database server - * shouldn't have allowed it. */ - VLOG_ERR("%s: database contains duplicate interface " - "name", iface_cfg->name); + * uniqueness constraints, so the database server + * shouldn't have allowed it. */ + VLOG_ERR("%s: database contains duplicate port name", + port_cfg->name); } continue; } - add_iface_to_cache(vsctl_ctx, port, iface_cfg); + if (port_is_fake_bridge(port_cfg) + && !sset_add(&bridges, port_cfg->name)) { + continue; + } + + port = add_port_to_cache(vsctl_ctx, br, port_cfg); + if (port) { + for (k = 0; k < port_cfg->n_interfaces; k++) { + struct ovsrec_interface *iface_cfg = + port_cfg->interfaces[k]; + struct vsctl_iface *iface; + + iface = shash_find_data(&vsctl_ctx->ifaces, + iface_cfg->name); + if (iface) { + if (iface_cfg == iface->iface_cfg) { + VLOG_WARN("%s: interface is in multiple ports " + "(%s and %s)", + iface_cfg->name, + iface->port->port_cfg->name, + port->port_cfg->name); + } else { + /* Log as an error because this violates the + * database's uniqueness constraints, so the + * database server shouldn't have allowed it. + */ + VLOG_ERR("%s: database contains duplicate " + "interface name", iface_cfg->name); + } + continue; + } + + add_iface_to_cache(vsctl_ctx, port, iface_cfg); + } + } } } } @@ -889,14 +906,23 @@ check_conflicts(struct vsctl_context *vsctl_ctx, const char *name, port = shash_find_data(&vsctl_ctx->ports, name); if (port) { - ctl_fatal("%s because a port named %s already exists on " - "bridge %s", msg, name, port->bridge->name); + if (port->bridge) { + ctl_fatal("%s because a port named %s already exists on " + "bridge %s", msg, name, port->bridge->name); + } else { + ctl_fatal("%s because a port named %s already exists", msg, name); + } } iface = shash_find_data(&vsctl_ctx->ifaces, name); if (iface) { - ctl_fatal("%s because an interface named %s already exists " - "on bridge %s", msg, name, iface->port->bridge->name); + if (iface->port->bridge) { + ctl_fatal("%s because an interface named %s already exists " + "on bridge %s", msg, name, iface->port->bridge->name); + } else { + ctl_fatal("%s because an interface named %s already exists", msg, + name); + } } free(msg); @@ -936,7 +962,7 @@ find_port(struct vsctl_context *vsctl_ctx, const char *name, bool must_exist) ovs_assert(vsctl_ctx->cache_valid); port = shash_find_data(&vsctl_ctx->ports, name); - if (port && !strcmp(name, port->bridge->name)) { + if (port && port->bridge && !strcmp(name, port->bridge->name)) { port = NULL; } if (must_exist && !port) { @@ -954,7 +980,8 @@ find_iface(struct vsctl_context *vsctl_ctx, const char *name, bool must_exist) ovs_assert(vsctl_ctx->cache_valid); iface = shash_find_data(&vsctl_ctx->ifaces, name); - if (iface && !strcmp(name, iface->port->bridge->name)) { + if (iface && iface->port->bridge && + !strcmp(name, iface->port->bridge->name)) { iface = NULL; } if (must_exist && !iface) { @@ -1679,14 +1706,16 @@ get_external_id(struct smap *smap, const char *prefix, const char *key, size_t prefix_len = strlen(prefix); size_t i; - for (i = 0; i < smap_count(smap); i++) { - const struct smap_node *node = sorted[i]; - if (!strncmp(node->key, prefix, prefix_len)) { - ds_put_format(output, "%s=%s\n", node->key + prefix_len, - node->value); + if (sorted) { + for (i = 0; i < smap_count(smap); i++) { + const struct smap_node *node = sorted[i]; + if (!strncmp(node->key, prefix, prefix_len)) { + ds_put_format(output, "%s=%s\n", node->key + prefix_len, + node->value); + } } + free(sorted); } - free(sorted); } } diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c index e5d99714d..356840089 100644 --- a/vtep/vtep-ctl.c +++ b/vtep/vtep-ctl.c @@ -250,8 +250,11 @@ parse_options(int argc, char *argv[], struct shash *local_options) exit(EXIT_SUCCESS); case 't': - if (!str_to_uint(optarg, 10, &timeout) || !timeout) { - ctl_fatal("value %s on -t or --timeout is invalid", optarg); + if (optarg) { + if (!str_to_uint(optarg, 10, &timeout) || !timeout) { + ctl_fatal("value %s on -t or --timeout is invalid", + optarg); + } } break; @@ -1065,42 +1068,46 @@ vtep_ctl_context_populate_cache(struct ctl_context *ctx) continue; } ps = shash_find_data(&vtepctl_ctx->pswitches, ps_cfg->name); - for (j = 0; j < ps_cfg->n_ports; j++) { - struct vteprec_physical_port *port_cfg = ps_cfg->ports[j]; - struct vtep_ctl_port *port; - size_t k; - - port = shash_find_data(&vtepctl_ctx->ports, port_cfg->name); - if (port) { - if (port_cfg == port->port_cfg) { - VLOG_WARN("%s: port is in multiple physical switches " - "(%s and %s)", - port_cfg->name, ps->name, port->ps->name); - } else { - /* Log as an error because this violates the database's - * uniqueness constraints, so the database server shouldn't - * have allowed it. */ - VLOG_ERR("%s: database contains duplicate port name", - port_cfg->name); + if (ps) { + for (j = 0; j < ps_cfg->n_ports; j++) { + struct vteprec_physical_port *port_cfg = ps_cfg->ports[j]; + struct vtep_ctl_port *port; + size_t k; + + port = shash_find_data(&vtepctl_ctx->ports, port_cfg->name); + if (port) { + if (port_cfg == port->port_cfg) { + VLOG_WARN("%s: port is in multiple physical switches " + "(%s and %s)", + port_cfg->name, ps->name, port->ps->name); + } else { + /* Log as an error because this violates the database's + * uniqueness constraints, so the database server + * shouldn't have allowed it. */ + VLOG_ERR("%s: database contains duplicate port name", + port_cfg->name); + } + continue; } - continue; - } - port = add_port_to_cache(vtepctl_ctx, ps, port_cfg); + port = add_port_to_cache(vtepctl_ctx, ps, port_cfg); + if (port) { + for (k = 0; k < port_cfg->n_vlan_bindings; k++) { + struct vtep_ctl_lswitch *ls; + char *vlan; - for (k = 0; k < port_cfg->n_vlan_bindings; k++) { - struct vtep_ctl_lswitch *ls; - char *vlan; + vlan = xasprintf("%"PRId64, + port_cfg->key_vlan_bindings[k]); + if (shash_find(&port->bindings, vlan)) { + ctl_fatal("multiple bindings for vlan %s", vlan); + } - vlan = xasprintf("%"PRId64, port_cfg->key_vlan_bindings[k]); - if (shash_find(&port->bindings, vlan)) { - ctl_fatal("multiple bindings for vlan %s", vlan); - } - - ls_cfg = port_cfg->value_vlan_bindings[k]; - ls = find_lswitch(vtepctl_ctx, ls_cfg->name, true); + ls_cfg = port_cfg->value_vlan_bindings[k]; + ls = find_lswitch(vtepctl_ctx, ls_cfg->name, true); - shash_add_nocopy(&port->bindings, vlan, ls); + shash_add_nocopy(&port->bindings, vlan, ls); + } + } } } } @@ -1892,8 +1899,10 @@ del_mcast_entry(struct ctl_context *ctx, vteprec_mcast_macs_remote_delete(mcast_mac->remote_cfg); } - free(node->data); - shash_delete(mcast_shash, node); + if (node) { + free(node->data); + shash_delete(mcast_shash, node); + } } else { if (local) { vteprec_mcast_macs_local_set_locator_set(mcast_mac->local_cfg,