summaryrefslogtreecommitdiff
path: root/include
diff options
context:
space:
mode:
authorDumitru Ceara <dceara@redhat.com>2022-04-11 13:37:50 +0200
committerIlya Maximets <i.maximets@ovn.org>2022-05-17 23:08:13 +0200
commit471babb8117aebbe42b2e1059c6f3c9d76fc11c1 (patch)
tree458e533e9bee73693a33de732e53deb74f42822f /include
parent3764f5188af07550d00f1e55fd4af6ef537d07de (diff)
downloadopenvswitch-471babb8117aebbe42b2e1059c6f3c9d76fc11c1.tar.gz
treewide: Avoid offsetting NULL pointers.
This is undefined behavior and was reported by UB Sanitizer: lib/meta-flow.c:3445:16: runtime error: member access within null pointer of type 'struct vl_mf_field' 0 0x6aad0f in mf_get_vl_mff lib/meta-flow.c:3445 1 0x6d96d7 in mf_from_oxm_header lib/nx-match.c:260 2 0x6d9e2e in nx_pull_header__ lib/nx-match.c:341 3 0x6daafa in nx_pull_header lib/nx-match.c:488 4 0x6abcb6 in mf_vl_mff_nx_pull_header lib/meta-flow.c:3605 5 0x73b9be in decode_NXAST_RAW_REG_MOVE lib/ofp-actions.c:2652 6 0x764ccd in ofpact_decode lib/ofp-actions.inc2:4681 [...] lib/sset.c:315:12: runtime error: applying zero offset to null pointer 0 0xcc2e6a in sset_at_position lib/sset.c:315:12 1 0x5734b3 in port_dump_next ofproto/ofproto-dpif.c:4083:20 [...] lib/ovsdb-data.c:2194:56: runtime error: applying zero offset to null pointer 0 0x5e9530 in ovsdb_datum_added_removed lib/ovsdb-data.c:2194:56 1 0x4d6258 in update_row_ref_count ovsdb/transaction.c:335:17 2 0x4c360b in for_each_txn_row ovsdb/transaction.c:1572:33 [...] lib/ofpbuf.c:440:30: runtime error: applying zero offset to null pointer 0 0x75066d in ofpbuf_push_uninit lib/ofpbuf.c:440 1 0x46ac8a in ovnacts_parse lib/actions.c:4190 2 0x46ad91 in ovnacts_parse_string lib/actions.c:4208 3 0x4106d1 in test_parse_actions tests/test-ovn.c:1324 [...] lib/ofp-actions.c:3205:22: runtime error: applying non-zero offset 2 to null pointer 0 0x6e1641 in set_field_split_str lib/ofp-actions.c:3205:22 [...] lib/tnl-ports.c:74:12: runtime error: applying zero offset to null pointer 0 0xceffe7 in tnl_port_cast lib/tnl-ports.c:74:12 1 0xcf14c3 in map_insert lib/tnl-ports.c:116:13 [...] ofproto/ofproto.c:8905:16: runtime error: applying zero offset to null pointer 0 0x556795 in eviction_group_hash_rule ofproto/ofproto.c:8905:16 1 0x503f8d in eviction_group_add_rule ofproto/ofproto.c:9022:42 [...] Also, it's valid to have an empty ofpact list and we should be able to try to iterate through it. UB Sanitizer report: include/openvswitch/ofp-actions.h:222:12: runtime error: applying zero offset to null pointer 0 0x665d69 in ofpact_end ./include/openvswitch/ofp-actions.h:222:12 1 0x66b2cf in ofpacts_put_openflow_actions lib/ofp-actions.c:8861:5 2 0x6ffdd1 in ofputil_encode_flow_mod lib/ofp-flow.c:447:9 [...] Signed-off-by: Dumitru Ceara <dceara@redhat.com> Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Diffstat (limited to 'include')
-rw-r--r--include/openvswitch/ofp-actions.h4
-rw-r--r--include/openvswitch/ofpbuf.h22
2 files changed, 20 insertions, 6 deletions
diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index 41bcb55d2..b7231c7bb 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -218,7 +218,9 @@ struct ofpact *ofpact_next_flattened(const struct ofpact *);
static inline struct ofpact *
ofpact_end(const struct ofpact *ofpacts, size_t ofpacts_len)
{
- return ALIGNED_CAST(struct ofpact *, (uint8_t *) ofpacts + ofpacts_len);
+ return ofpacts
+ ? ALIGNED_CAST(struct ofpact *, (uint8_t *) ofpacts + ofpacts_len)
+ : NULL;
}
static inline bool
diff --git a/include/openvswitch/ofpbuf.h b/include/openvswitch/ofpbuf.h
index 1136ba04c..32f03ea83 100644
--- a/include/openvswitch/ofpbuf.h
+++ b/include/openvswitch/ofpbuf.h
@@ -179,7 +179,11 @@ static inline void ofpbuf_delete(struct ofpbuf *b)
static inline void *ofpbuf_at(const struct ofpbuf *b, size_t offset,
size_t size)
{
- return offset + size <= b->size ? (char *) b->data + offset : NULL;
+ if (offset + size <= b->size) {
+ ovs_assert(b->data);
+ return (char *) b->data + offset;
+ }
+ return NULL;
}
/* Returns a pointer to byte 'offset' in 'b', which must contain at least
@@ -188,20 +192,23 @@ static inline void *ofpbuf_at_assert(const struct ofpbuf *b, size_t offset,
size_t size)
{
ovs_assert(offset + size <= b->size);
- return ((char *) b->data) + offset;
+ ovs_assert(b->data);
+ return (char *) b->data + offset;
}
/* Returns a pointer to byte following the last byte of data in use in 'b'. */
static inline void *ofpbuf_tail(const struct ofpbuf *b)
{
- return (char *) b->data + b->size;
+ ovs_assert(b->data || !b->size);
+ return b->data ? (char *) b->data + b->size : NULL;
}
/* Returns a pointer to byte following the last byte allocated for use (but
* not necessarily in use) in 'b'. */
static inline void *ofpbuf_end(const struct ofpbuf *b)
{
- return (char *) b->base + b->allocated;
+ ovs_assert(b->base || !b->allocated);
+ return b->base ? (char *) b->base + b->allocated : NULL;
}
/* Returns the number of bytes of headroom in 'b', that is, the number of bytes
@@ -249,6 +256,11 @@ static inline void *ofpbuf_pull(struct ofpbuf *b, size_t size)
{
ovs_assert(b->size >= size);
void *data = b->data;
+
+ if (!size) {
+ return data;
+ }
+
b->data = (char*)b->data + size;
b->size = b->size - size;
return data;
@@ -270,7 +282,7 @@ static inline struct ofpbuf *ofpbuf_from_list(const struct ovs_list *list)
static inline bool ofpbuf_equal(const struct ofpbuf *a, const struct ofpbuf *b)
{
return a->size == b->size &&
- memcmp(a->data, b->data, a->size) == 0;
+ (a->size == 0 || memcmp(a->data, b->data, a->size) == 0);
}
static inline bool ofpbuf_oversized(const struct ofpbuf *ofpacts)