diff mbox series

[ovs-dev,v16,2/2] lib, ovsdb, vtep: Add various null pointer checks

Message ID 20230903152155.753998-3-jamestiotio@gmail.com
State Accepted
Commit 880a2bbb4b90c64b5d02dc5f5b16e046caa7ed87
Headers show
Series treewide: Fix multiple Coverity defects | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

James Raphael Tiovalen Sept. 3, 2023, 3:21 p.m. UTC 
This commit adds various null pointer checks to some files in the `lib`,
`ovsdb`, and `vtep` directories to fix several Coverity defects. These
changes are grouped together as they perform similar checks, returning
early, skipping some action, or logging a warning if a null pointer is
encountered.

Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
---
 lib/dp-packet.c        |  8 ++++----
 lib/dpctl.c            | 12 ++++++++++++
 lib/shash.c            |  4 ++++
 lib/sset.c             |  5 +++++
 ovsdb/jsonrpc-server.c |  2 +-
 ovsdb/monitor.c        |  3 +++
 ovsdb/ovsdb-client.c   |  6 ++++--
 ovsdb/row.c            |  5 ++++-
 vtep/vtep-ctl.c        | 17 +++++++++--------
 9 files changed, 46 insertions(+), 16 deletions(-)

Comments

Eelco Chaudron Sept. 15, 2023, 1:20 p.m. UTC | #1 
On 3 Sep 2023, at 17:21, James Raphael Tiovalen wrote:

> This commit adds various null pointer checks to some files in the `lib`,
> `ovsdb`, and `vtep` directories to fix several Coverity defects. These
> changes are grouped together as they perform similar checks, returning
> early, skipping some action, or logging a warning if a null pointer is
> encountered.
>
> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>

Thanks James for following through with this patchset!

This revision looks all good to me!

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Ilya Maximets Sept. 25, 2023, 12:18 p.m. UTC | #2 
On 9/15/23 15:20, Eelco Chaudron wrote:
> 
> 
> On 3 Sep 2023, at 17:21, James Raphael Tiovalen wrote:
> 
>> This commit adds various null pointer checks to some files in the `lib`,
>> `ovsdb`, and `vtep` directories to fix several Coverity defects. These
>> changes are grouped together as they perform similar checks, returning
>> early, skipping some action, or logging a warning if a null pointer is
>> encountered.
>>
>> Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
>> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> 
> Thanks James for following through with this patchset!
> 
> This revision looks all good to me!
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>

Applied.  Thanks!

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 8bc747c10..ed004c3b9 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -360,7 +360,7 @@  void *
 dp_packet_put_zeros(struct dp_packet *b, size_t size)
 {
     void *dst = dp_packet_put_uninit(b, size);
-    memset(dst, 0, size);
+    nullable_memset(dst, 0, size);
     return dst;
 }
 
@@ -371,7 +371,7 @@  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);
+    nullable_memcpy(dst, p, size);
     return dst;
 }
 
@@ -443,7 +443,7 @@  void *
 dp_packet_push_zeros(struct dp_packet *b, size_t size)
 {
     void *dst = dp_packet_push_uninit(b, size);
-    memset(dst, 0, size);
+    nullable_memset(dst, 0, size);
     return dst;
 }
 
@@ -454,7 +454,7 @@  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);
+    nullable_memcpy(dst, p, size);
     return dst;
 }
 
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 79b82a176..cd12625a1 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -336,6 +336,12 @@  dpctl_add_if(int argc OVS_UNUSED, const char *argv[],
                 value = "";
             }
 
+            if (!key) {
+                dpctl_error(dpctl_p, 0, "Invalid option format");
+                error = EINVAL;
+                goto next;
+            }
+
             if (!strcmp(key, "type")) {
                 type = value;
             } else if (!strcmp(key, "port_no")) {
@@ -454,6 +460,12 @@  dpctl_set_if(int argc, const char *argv[], struct dpctl_params *dpctl_p)
                 value = "";
             }
 
+            if (!key) {
+                dpctl_error(dpctl_p, 0, "Invalid option format");
+                error = EINVAL;
+                goto next_destroy_args;
+            }
+
             if (!strcmp(key, "type")) {
                 if (strcmp(value, type)) {
                     dpctl_error(dpctl_p, 0,
diff --git a/lib/shash.c b/lib/shash.c
index 6af985d0b..92260cddf 100644
--- a/lib/shash.c
+++ b/lib/shash.c
@@ -205,6 +205,10 @@  shash_delete(struct shash *sh, struct shash_node *node)
 char *
 shash_steal(struct shash *sh, struct shash_node *node)
 {
+    if (!node) {
+        return NULL;
+    }
+
     char *name = node->name;
 
     hmap_remove(&sh->map, &node->node);
diff --git a/lib/sset.c b/lib/sset.c
index aa1790020..fda268129 100644
--- a/lib/sset.c
+++ b/lib/sset.c
@@ -261,6 +261,11 @@  char *
 sset_pop(struct sset *set)
 {
     const char *name = SSET_FIRST(set);
+
+    if (!name) {
+        return NULL;
+    }
+
     char *copy = xstrdup(name);
     sset_delete(set, SSET_NODE_FROM_NAME(name));
     return copy;
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 9a77760c3..a3ca48a7b 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);
         }
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 9829cd39c..4ccb51b1a 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -483,6 +483,7 @@  ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
     struct ovsdb_monitor_column *c;
 
     mt = shash_find_data(&dbmon->tables, table->schema->name);
+    ovs_assert(mt);
 
     /* Check for column duplication. Return duplicated column name. */
     if (mt->columns_index_map[column->index] != -1) {
@@ -813,6 +814,8 @@  ovsdb_monitor_table_condition_update(
     struct ovsdb_error *error;
     struct ovsdb_condition cond = OVSDB_CONDITION_INITIALIZER(&cond);
 
+    ovs_assert(mtc);
+
     error = ovsdb_condition_from_json(table->schema, cond_json,
                                       NULL, &cond);
     if (error) {
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 46484630d..7249805ba 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -1843,7 +1843,7 @@  do_dump(struct jsonrpc *rpc, const char *database,
     struct ovsdb_schema *schema;
     struct json *transaction;
 
-    const struct shash_node *node, **tables;
+    const struct shash_node *node, **tables = NULL;
     size_t n_tables;
     struct ovsdb_table_schema *tschema;
     const struct shash *columns;
@@ -1869,8 +1869,10 @@  do_dump(struct jsonrpc *rpc, const char *database,
             shash_add(&custom_columns, argv[i], node->data);
         }
     } else {
-        tables = shash_sort(&schema->tables);
         n_tables = shash_count(&schema->tables);
+        if (n_tables) {
+            tables = shash_sort(&schema->tables);
+        }
     }
 
     /* Construct transaction to retrieve entire database. */
diff --git a/ovsdb/row.c b/ovsdb/row.c
index 2b52b6816..6b52509a9 100644
--- a/ovsdb/row.c
+++ b/ovsdb/row.c
@@ -406,7 +406,10 @@  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/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index 61ec4801e..26b8540b4 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -1859,18 +1859,21 @@  del_mcast_entry(struct ctl_context *ctx,
                 const char *encap, const char *dst_ip, bool local)
 {
     struct vtep_ctl_context *vtepctl_ctx = vtep_ctl_context_cast(ctx);
+    struct vteprec_physical_locator_set *ploc_set_cfg;
+    struct vteprec_physical_locator *ploc_cfg;
     struct vtep_ctl_mcast_mac *mcast_mac;
     struct shash *mcast_shash;
-    struct vteprec_physical_locator *ploc_cfg;
-    struct vteprec_physical_locator_set *ploc_set_cfg;
+    struct shash_node *mcast_node;
 
     mcast_shash = local ? &ls->mcast_local : &ls->mcast_remote;
 
-    mcast_mac = shash_find_data(mcast_shash, mac);
-    if (!mcast_mac) {
+    mcast_node = shash_find(mcast_shash, mac);
+    if (!mcast_node || !mcast_node->data) {
         return;
     }
 
+    mcast_mac = mcast_node->data;
+
     ploc_cfg = find_ploc(vtepctl_ctx, encap, dst_ip);
     if (!ploc_cfg) {
         /* Couldn't find the physical locator, so just ignore. */
@@ -1883,8 +1886,6 @@  del_mcast_entry(struct ctl_context *ctx,
 
     del_ploc_from_mcast_mac(mcast_mac, ploc_cfg);
     if (ovs_list_is_empty(&mcast_mac->locators)) {
-        struct shash_node *node = shash_find(mcast_shash, mac);
-
         vteprec_physical_locator_set_delete(ploc_set_cfg);
 
         if (local) {
@@ -1893,8 +1894,8 @@  del_mcast_entry(struct ctl_context *ctx,
             vteprec_mcast_macs_remote_delete(mcast_mac->remote_cfg);
         }
 
-        free(node->data);
-        shash_delete(mcast_shash, node);
+        free(mcast_node->data);
+        shash_delete(mcast_shash, mcast_node);
     } else {
         if (local) {
             vteprec_mcast_macs_local_set_locator_set(mcast_mac->local_cfg,