diff options
author | Dumitru Ceara <dceara@redhat.com> | 2022-04-11 13:37:50 +0200 |
---|---|---|
committer | Ilya Maximets <i.maximets@ovn.org> | 2022-05-17 23:08:13 +0200 |
commit | 471babb8117aebbe42b2e1059c6f3c9d76fc11c1 (patch) | |
tree | 458e533e9bee73693a33de732e53deb74f42822f /include | |
parent | 3764f5188af07550d00f1e55fd4af6ef537d07de (diff) | |
download | openvswitch-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.h | 4 | ||||
-rw-r--r-- | include/openvswitch/ofpbuf.h | 22 |
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) |