This allow easy bitmap resizing to avoid the following address sanitizer
crashes if we are exceeding bitmap allocated size. This issue can occurs
since IP lflow support for logical routers does not take into account
ovn_lflow bitmap resize.
=================================================================
==52991==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x7bce82430100 at pc 0x0000004eae83 bp 0x7ffd7306f980 sp 0x7ffd7306f978
READ of size 8 at 0x7bce82430100 thread T0
#0 0x0000004eae82 in bitmap_is_set ovn/ovs/lib/bitmap.h:91
#1 0x0000004eae82 in lflow_table_add_lflow northd/lflow-mgr.c:765
#2 0x00000046c075 in build_adm_ctrl_flows_for_lrouter northd/northd.c:13759
#3 0x00000046c075 in build_lswitch_and_lrouter_iterate_by_lr
northd/northd.c:18591
#4 0x000000479805 in lflow_handle_northd_lr_changes northd/northd.c:19299
#5 0x00000049bcc0 in lflow_northd_handler northd/en-lflow.c:158
#6 0x000000508895 in engine_compute lib/inc-proc-eng.c:474
#7 0x000000508895 in engine_run_node lib/inc-proc-eng.c:546
#8 0x000000508895 in engine_run lib/inc-proc-eng.c:575
#9 0x0000004e0cff in inc_proc_northd_run northd/inc-proc-northd.c:602
#10 0x0000004077b4 in main northd/ovn-northd.c:1104
#11 0x7fae83523574 in __libc_start_call_main (/lib64/libc.so.6+0x3574)
(BuildId: 48c4b9b1efb1df15da8e787f489128bf31893317)
#12 0x7fae83523627 in __libc_start_main@GLIBC_2.2.5
(/lib64/libc.so.6+0x3627) (BuildId: 48c4b9b1efb1df15da8e787f489128bf31893317)
#13 0x00000040ae74 in _start (ovn/northd/ovn-northd+0x40ae74) (BuildId:
5373e3f3823362d2c1a220082395cf8a9aae28bc)
0x7bce82430100 is located 0 bytes after 16-byte region
[0x7bce824300f0,0x7bce82430100)
allocated by thread T0 here:
#0 0x7fae83ee68a3 in calloc (/lib64/libasan.so.8+0xe68a3) (BuildId:
cab80046dbc1c97c6e14490acc37d079701f8d9a)
#1 0x00000077873e in xcalloc__ lib/util.c:125
#2 0x00000077873e in xzalloc__ lib/util.c:135
#3 0x00000077873e in xzalloc lib/util.c:169
#4 0x0000004ea05b in bitmap_allocate ovn/ovs/lib/bitmap.h:51
#5 0x0000004ea05b in ovn_lflow_init northd/lflow-mgr.c:901
#6 0x0000004ea05b in do_ovn_lflow_add northd/lflow-mgr.c:1028
#7 0x0000004ea05b in lflow_table_add_lflow northd/lflow-mgr.c:730
#8 0x00000046c075 in build_adm_ctrl_flows_for_lrouter northd/northd.c:13759
#9 0x00000046c075 in build_lswitch_and_lrouter_iterate_by_lr
northd/northd.c:18591
#10 0x000000479805 in lflow_handle_northd_lr_changes northd/northd.c:19299
#11 0x00000049bcc0 in lflow_northd_handler northd/en-lflow.c:158
#12 0x000000508895 in engine_compute lib/inc-proc-eng.c:474
#13 0x000000508895 in engine_run_node lib/inc-proc-eng.c:546
#14 0x000000508895 in engine_run lib/inc-proc-eng.c:575
#15 0x0000004e0cff in inc_proc_northd_run northd/inc-proc-northd.c:602
#16 0x0000004077b4 in main northd/ovn-northd.c:1104
#17 0x7fae83523574 in __libc_start_call_main (/lib64/libc.so.6+0x3574)
(BuildId: 48c4b9b1efb1df15da8e787f489128bf31893317)
#18 0x7fae83523627 in __libc_start_main@GLIBC_2.2.5
(/lib64/libc.so.6+0x3627) (BuildId: 48c4b9b1efb1df15da8e787f489128bf31893317)
#19 0x00000040ae74 in _start (ovn/northd/ovn-northd+0x40ae74) (BuildId:
5373e3f3823362d2c1a220082395cf8a9aae28bc)
Fixes: 9ec96d0d85b6 ("northd: Add and delete logical routers in en-lflow engine
node.")
Fixes: f22a005f8935 ("northd: Convert datapath array into vector.")
Reported-at: https://issues.redhat.com/browse/FDP-2643
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
northd/lflow-mgr.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index e60f248f3..3af36c7fd 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -173,7 +173,7 @@ struct ovn_lflow {
struct hmap_node hmap_node;
struct ovn_datapath *od; /* 'logical_datapath' in SB schema. */
- unsigned long *dpg_bitmap; /* Bitmap of all datapaths by their 'index'.*/
+ struct dynamic_bitmap dpg_bitmap;
enum ovn_stage stage;
uint16_t priority;
char *match;
@@ -636,13 +636,13 @@ lflow_ref_unlink_lflows(struct lflow_ref *lflow_ref)
BITMAP_FOR_EACH_1 (index, lrn->dpgrp_bitmap_len,
lrn->dpgrp_bitmap) {
if (dp_refcnt_release(&lrn->lflow->dp_refcnts_map, index)) {
- bitmap_set0(lrn->lflow->dpg_bitmap, index);
+ dynamic_bitmap_set0(&lrn->lflow->dpg_bitmap, index);
}
}
} else {
if (dp_refcnt_release(&lrn->lflow->dp_refcnts_map,
lrn->dp_index)) {
- bitmap_set0(lrn->lflow->dpg_bitmap, lrn->dp_index);
+ dynamic_bitmap_set0(&lrn->lflow->dpg_bitmap, lrn->dp_index);
}
}
@@ -756,13 +756,13 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,
size_t index;
BITMAP_FOR_EACH_1 (index, dp_bitmap_len, dp_bitmap) {
/* Allocate a reference counter only if already used. */
- if (bitmap_is_set(lflow->dpg_bitmap, index)) {
+ if (bitmap_is_set(lflow->dpg_bitmap.map, index)) {
dp_refcnt_use(&lflow->dp_refcnts_map, index);
}
}
} else {
/* Allocate a reference counter only if already used. */
- if (bitmap_is_set(lflow->dpg_bitmap, lrn->dp_index)) {
+ if (bitmap_is_set(lflow->dpg_bitmap.map, lrn->dp_index)) {
dp_refcnt_use(&lflow->dp_refcnts_map, lrn->dp_index);
}
}
@@ -898,7 +898,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath
*od,
char *stage_hint, const char *where,
const char *flow_desc, struct uuid sbuuid)
{
- lflow->dpg_bitmap = bitmap_allocate(dp_bitmap_len);
+ dynamic_bitmap_alloc(&lflow->dpg_bitmap, dp_bitmap_len);
lflow->od = od;
lflow->stage = stage;
lflow->priority = priority;
@@ -982,7 +982,7 @@ static void
ovn_lflow_destroy(struct lflow_table *lflow_table, struct ovn_lflow *lflow)
{
hmap_remove(&lflow_table->entries, &lflow->hmap_node);
- bitmap_free(lflow->dpg_bitmap);
+ dynamic_bitmap_free(&lflow->dpg_bitmap);
free(lflow->match);
free(lflow->actions);
free(lflow->io_port);
@@ -1014,6 +1014,7 @@ do_ovn_lflow_add(struct lflow_table *lflow_table, size_t
dp_bitmap_len,
old_lflow = ovn_lflow_find(&lflow_table->entries, stage,
priority, match, actions, ctrl_meter, hash);
if (old_lflow) {
+ dynamic_bitmap_realloc(&old_lflow->dpg_bitmap, dp_bitmap_len);
if (old_lflow->sync_state != LFLOW_STALE) {
return old_lflow;
}
@@ -1074,14 +1075,13 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
is_switch = false;
}
- lflow->n_ods = bitmap_count1(lflow->dpg_bitmap, n_datapaths);
+ lflow->n_ods = bitmap_count1(lflow->dpg_bitmap.map, n_datapaths);
ovs_assert(lflow->n_ods);
if (lflow->n_ods == 1) {
/* There is only one datapath, so it should be moved out of the
* group to a single 'od'. */
- size_t index = bitmap_scan(lflow->dpg_bitmap, true, 0,
- n_datapaths);
+ size_t index = dynamic_bitmap_scan(&lflow->dpg_bitmap, true, 0);
lflow->od = datapaths_array[index];
lflow->dpg = NULL;
@@ -1180,7 +1180,7 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
} else {
sbrec_logical_flow_set_logical_datapath(sbflow, NULL);
lflow->dpg = ovn_dp_group_get(dp_groups, lflow->n_ods,
- lflow->dpg_bitmap,
+ lflow->dpg_bitmap.map,
n_datapaths);
if (lflow->dpg) {
/* Update the dpg's sb dp_group. */
@@ -1213,7 +1213,7 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
} else {
lflow->dpg = ovn_dp_group_create(
ovnsb_txn, dp_groups, sbrec_dp_group,
- lflow->n_ods, lflow->dpg_bitmap,
+ lflow->n_ods, lflow->dpg_bitmap.map,
n_datapaths, is_switch,
ls_datapaths,
lr_datapaths);
@@ -1312,10 +1312,10 @@ ovn_dp_group_add_with_reference(struct ovn_lflow
*lflow_ref,
OVS_REQUIRES(fake_hash_mutex)
{
if (od) {
- bitmap_set1(lflow_ref->dpg_bitmap, od->index);
+ dynamic_bitmap_set1(&lflow_ref->dpg_bitmap, od->index);
}
if (dp_bitmap) {
- bitmap_or(lflow_ref->dpg_bitmap, dp_bitmap, bitmap_len);
+ bitmap_or(lflow_ref->dpg_bitmap.map, dp_bitmap, bitmap_len);
}
}
@@ -1347,7 +1347,7 @@ lflow_ref_sync_lflows__(struct lflow_ref *lflow_ref,
n_datapaths = ods_size(lr_datapaths);
}
- size_t n_ods = bitmap_count1(lflow->dpg_bitmap, n_datapaths);
+ size_t n_ods = bitmap_count1(lflow->dpg_bitmap.map, n_datapaths);
if (n_ods) {
if (!sync_lflow_to_sb(lflow, ovnsb_txn, lflow_table, ls_datapaths,
--
2.51.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
