diff mbox series

[ovs-dev,02/11] treewide: Don't pass NULL to library functions that expect non-NULL.

Message ID 20211217131658.30994.78806.stgit@dceara.remote.csb
State Superseded
Headers show
Series Fix UndefinedBehaviorSanitizer reported issues and enable it in CI. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Dumitru Ceara Dec. 17, 2021, 1:16 p.m. UTC 
It's actually undefined behavior to pass NULL to standard library
functions that manipulate arrays (e.g., qsort, memcpy, memcmp), even if
the passed number of items is 0.

UB Sanitizer reports:
  ovsdb/monitor.c:408:9: runtime error: null pointer passed as argument 1, which is declared to never be null
      #0 0x406ae1 in ovsdb_monitor_columns_sort ovsdb/monitor.c:408
      #1 0x406ae1 in ovsdb_monitor_add ovsdb/monitor.c:1683
  [...]
  lib/ovsdb-data.c:1970:5: runtime error: null pointer passed as argument 2, which is declared to never be null
      #0 0x4071c8 in ovsdb_datum_push_unsafe lib/ovsdb-data.c:1970
      #1 0x471cd0 in ovsdb_datum_apply_diff_in_place lib/ovsdb-data.c:2345
  [...]
  ofproto/ofproto-dpif-rid.c:159:17: runtime error: null pointer passed as argument 1, which is declared to never be null
      #0 0x4df5d8 in frozen_state_equal ofproto/ofproto-dpif-rid.c:159
      #1 0x4dfd27 in recirc_find_equal ofproto/ofproto-dpif-rid.c:179
      [...]

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 lib/dpif-netdev.c          |    4 +++-
 lib/ofp-actions.c          |    2 +-
 lib/ofpbuf.c               |    4 ++++
 lib/ovsdb-data.c           |    4 ++++
 ofproto/ofproto-dpif-rid.c |    6 ++++--
 ovsdb/monitor.c            |    4 ++++
 6 files changed, 20 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a790df5fd697..f62202a8b53d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4506,8 +4506,10 @@  dp_netdev_actions_create(const struct nlattr *actions, size_t size)
     struct dp_netdev_actions *netdev_actions;
 
     netdev_actions = xmalloc(sizeof *netdev_actions + size);
-    memcpy(netdev_actions->actions, actions, size);
     netdev_actions->size = size;
+    if (size) {
+        memcpy(netdev_actions->actions, actions, size);
+    }
 
     return netdev_actions;
 }
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index ecf914eac10b..f21b3c579f2b 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -9147,7 +9147,7 @@  bool
 ofpacts_equal(const struct ofpact *a, size_t a_len,
               const struct ofpact *b, size_t b_len)
 {
-    return a_len == b_len && !memcmp(a, b, a_len);
+    return a_len == b_len && (!a_len || !memcmp(a, b, a_len));
 }
 
 /* Returns true if the 'a_len' bytes of actions in 'a' and the 'b_len' bytes of
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 4edb3c114ae6..271105bdea17 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -384,6 +384,10 @@  ofpbuf_put_zeros(struct ofpbuf *b, size_t size)
 void *
 ofpbuf_put(struct ofpbuf *b, const void *p, size_t size)
 {
+    if (!size) {
+        return ofpbuf_tail(b);
+    }
+
     void *dst = ofpbuf_put_uninit(b, size);
     memcpy(dst, p, size);
     return dst;
diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index 2832e94ea04a..6b1c20ff85a0 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -1967,6 +1967,10 @@  ovsdb_datum_push_unsafe(struct ovsdb_datum *dst,
                         unsigned int start_idx, unsigned int n,
                         const struct ovsdb_type *type)
 {
+    if (n == 0) {
+        return;
+    }
+
     memcpy(&dst->keys[dst->n], &src->keys[start_idx], n * sizeof src->keys[0]);
     if (type->value.type != OVSDB_TYPE_VOID) {
         memcpy(&dst->values[dst->n], &src->values[start_idx],
diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index 29aafc2c0b40..f01468025c03 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -156,7 +156,7 @@  frozen_state_equal(const struct frozen_state *a, const struct frozen_state *b)
             && uuid_equals(&a->ofproto_uuid, &b->ofproto_uuid)
             && !memcmp(&a->metadata, &b->metadata, sizeof a->metadata)
             && a->stack_size == b->stack_size
-            && !memcmp(a->stack, b->stack, a->stack_size)
+            && (!a->stack_size || !memcmp(a->stack, b->stack, a->stack_size))
             && a->mirrors == b->mirrors
             && a->conntracked == b->conntracked
             && a->was_mpls == b->was_mpls
@@ -164,7 +164,9 @@  frozen_state_equal(const struct frozen_state *a, const struct frozen_state *b)
                              b->ofpacts, b->ofpacts_len)
             && ofpacts_equal(a->action_set, a->action_set_len,
                              b->action_set, b->action_set_len)
-            && !memcmp(a->userdata, b->userdata, a->userdata_len)
+            && a->userdata_len == b->userdata_len
+            && (!a->userdata_len
+                || !memcmp(a->userdata, b->userdata, a->userdata_len))
             && uuid_equals(&a->xport_uuid, &b->xport_uuid));
 }
 
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index ab814cf20e26..0f222cc99284 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -405,6 +405,10 @@  ovsdb_monitor_columns_sort(struct ovsdb_monitor *dbmon)
     SHASH_FOR_EACH (node, &dbmon->tables) {
         struct ovsdb_monitor_table *mt = node->data;
 
+        if (mt->n_columns == 0) {
+            continue;
+        }
+
         qsort(mt->columns, mt->n_columns, sizeof *mt->columns,
               compare_ovsdb_monitor_column);
         for (i = 0; i < mt->n_columns; i++) {