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_dp_group 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]>
---
lib/ovn-util.h | 21 ++++++++++++++-
northd/en-sync-sb.c | 12 +++------
northd/lflow-mgr.c | 62 ++++++++++++++++++++-------------------------
northd/lflow-mgr.h | 14 +++++-----
tests/ovn-northd.at | 34 +++++++++++++++++++++++++
5 files changed, 93 insertions(+), 50 deletions(-)
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 148043037..875810860 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -603,11 +603,30 @@ dynamic_bitmap_or(struct dynamic_bitmap *db,
}
static inline unsigned long *
-dynamic_bitmap_clone_map(struct dynamic_bitmap *db)
+dynamic_bitmap_clone_map(const struct dynamic_bitmap *db)
{
return bitmap_clone(db->map, db->capacity);
}
+static inline bool
+dynamic_bitmap_equal(const struct dynamic_bitmap *a,
+ const struct dynamic_bitmap *b)
+{
+ return bitmap_equal(a->map, b->map, MIN(a->capacity, b->capacity));
+}
+
+static inline void
+dynamic_bitmap_clone_from_db(struct dynamic_bitmap *dst,
+ const struct dynamic_bitmap *orig)
+{
+ if (dst->map) {
+ bitmap_free(dst->map);
+ }
+ dst->map = dynamic_bitmap_clone_map(orig);
+ dst->n_elems = dynamic_bitmap_count1(orig);
+ dst->capacity = orig->capacity;
+}
+
static inline size_t
dynamic_bitmap_scan(struct dynamic_bitmap *dp, bool target, size_t start)
{
diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index cf18d6e90..b257e8f10 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -780,8 +780,7 @@ sync_sb_lb_record(struct sb_lb_record *sb_lb,
if (!dynamic_bitmap_is_empty(&lb_dps->nb_ls_map)) {
sb_lb->ls_dpg = ovn_dp_group_get(&sb_lbs->ls_dp_groups,
- lb_dps->nb_ls_map.n_elems,
- lb_dps->nb_ls_map.map,
+ &lb_dps->nb_ls_map,
ods_size(ls_datapaths));
if (sb_lb->ls_dpg) {
/* Update the dpg's sb dp_group. */
@@ -811,8 +810,7 @@ sync_sb_lb_record(struct sb_lb_record *sb_lb,
} else {
sb_lb->ls_dpg = ovn_dp_group_create(
ovnsb_txn, &sb_lbs->ls_dp_groups, sbrec_ls_dp_group,
- lb_dps->nb_ls_map.n_elems, &lb_dps->nb_ls_map,
- true, ls_datapaths, lr_datapaths);
+ &lb_dps->nb_ls_map, true, ls_datapaths, lr_datapaths);
}
if (chassis_features->ls_dpg_column) {
@@ -834,8 +832,7 @@ sync_sb_lb_record(struct sb_lb_record *sb_lb,
if (!dynamic_bitmap_is_empty(&lb_dps->nb_lr_map)) {
sb_lb->lr_dpg = ovn_dp_group_get(&sb_lbs->lr_dp_groups,
- lb_dps->nb_lr_map.n_elems,
- lb_dps->nb_lr_map.map,
+ &lb_dps->nb_lr_map,
ods_size(lr_datapaths));
if (sb_lb->lr_dpg) {
/* Update the dpg's sb dp_group. */
@@ -865,8 +862,7 @@ sync_sb_lb_record(struct sb_lb_record *sb_lb,
} else {
sb_lb->lr_dpg = ovn_dp_group_create(
ovnsb_txn, &sb_lbs->lr_dp_groups, sbrec_lr_dp_group,
- lb_dps->nb_lr_map.n_elems, &lb_dps->nb_lr_map,
- false, ls_datapaths, lr_datapaths);
+ &lb_dps->nb_lr_map, false, ls_datapaths, lr_datapaths);
}
sbrec_load_balancer_set_lr_datapath_group(sbrec_lb,
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index 38839bd5d..a942f287b 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -67,10 +67,10 @@ static struct sbrec_logical_dp_group
*ovn_sb_insert_or_update_logical_dp_group(
struct sbrec_logical_dp_group *,
const unsigned long *dpg_bitmap,
const struct ovn_datapaths *);
-static struct ovn_dp_group *ovn_dp_group_find(const struct hmap *dp_groups,
- const unsigned long *dpg_bitmap,
- size_t bitmap_len,
- uint32_t hash);
+static struct ovn_dp_group *ovn_dp_group_find(
+ const struct hmap *dp_groups,
+ const struct dynamic_bitmap *dpg_bitmap,
+ size_t bitmap_len, uint32_t hash);
static void ovn_dp_group_use(struct ovn_dp_group *);
static void ovn_dp_group_release(struct hmap *dp_groups,
struct ovn_dp_group *);
@@ -181,8 +181,6 @@ struct ovn_lflow {
char *io_port;
char *stage_hint;
char *ctrl_meter;
- size_t n_ods; /* Number of datapaths referenced by 'od' and
- * 'dpg_bitmap'. */
struct ovn_dp_group *dpg; /* Link to unique Sb datapath group. */
const char *where;
const char *flow_desc;
@@ -776,13 +774,13 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,
}
struct ovn_dp_group *
-ovn_dp_group_get(struct hmap *dp_groups, size_t desired_n,
- const unsigned long *desired_bitmap,
+ovn_dp_group_get(struct hmap *dp_groups,
+ const struct dynamic_bitmap *desired_bitmap,
size_t bitmap_len)
{
uint32_t hash;
- hash = hash_int(desired_n, 0);
+ hash = hash_int(desired_bitmap->n_elems, 0);
return ovn_dp_group_find(dp_groups, desired_bitmap, bitmap_len, hash);
}
@@ -794,7 +792,6 @@ struct ovn_dp_group *
ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn,
struct hmap *dp_groups,
struct sbrec_logical_dp_group *sb_group,
- size_t desired_n,
const struct dynamic_bitmap *desired_bitmap,
bool is_switch,
const struct ovn_datapaths *ls_datapaths,
@@ -803,10 +800,10 @@ ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn,
struct ovn_dp_group *dpg;
bool update_dp_group = false, can_modify = false;
- unsigned long *dpg_bitmap;
- size_t i, n = 0;
+ struct dynamic_bitmap dpg_bitmap;
+ size_t i;
- dpg_bitmap = sb_group ? bitmap_allocate(desired_bitmap->capacity) : NULL;
+ dynamic_bitmap_alloc(&dpg_bitmap, desired_bitmap->capacity);
for (i = 0; sb_group && i < sb_group->n_datapaths; i++) {
struct ovn_datapath *datapath_od;
@@ -817,27 +814,25 @@ ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn,
if (!datapath_od || ovn_datapath_is_stale(datapath_od)) {
break;
}
- bitmap_set1(dpg_bitmap, datapath_od->index);
- n++;
+ dynamic_bitmap_set1(&dpg_bitmap, datapath_od->index);
}
if (!sb_group || i != sb_group->n_datapaths) {
/* No group or stale group. Not going to be used. */
update_dp_group = true;
can_modify = true;
- } else if (!bitmap_equal(dpg_bitmap, desired_bitmap->map,
- desired_bitmap->capacity)) {
+ } else if (!dynamic_bitmap_equal(&dpg_bitmap, desired_bitmap)) {
/* The group in Sb is different. */
update_dp_group = true;
/* We can modify existing group if it's not already in use. */
- can_modify = !ovn_dp_group_find(dp_groups, dpg_bitmap,
+ can_modify = !ovn_dp_group_find(dp_groups, &dpg_bitmap,
desired_bitmap->capacity,
- hash_int(n, 0));
+ hash_int(dpg_bitmap.n_elems, 0));
}
- bitmap_free(dpg_bitmap);
+ dynamic_bitmap_free(&dpg_bitmap);
dpg = xzalloc(sizeof *dpg);
- dpg->bitmap = bitmap_clone(desired_bitmap->map, desired_bitmap->capacity);
+ dynamic_bitmap_clone_from_db(&dpg->bitmap, desired_bitmap);
if (!update_dp_group) {
dpg->dp_group = sb_group;
} else {
@@ -848,7 +843,7 @@ ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn,
is_switch ? ls_datapaths : lr_datapaths);
}
dpg->dpg_uuid = dpg->dp_group->header_.uuid;
- hmap_insert(dp_groups, &dpg->node, hash_int(desired_n, 0));
+ hmap_insert(dp_groups, &dpg->node, hash_int(desired_bitmap->n_elems, 0));
return dpg;
}
@@ -1076,10 +1071,9 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
is_switch = false;
}
- lflow->n_ods = dynamic_bitmap_count1(&lflow->dpg_bitmap);
- ovs_assert(lflow->n_ods);
-
- if (lflow->n_ods == 1) {
+ size_t n_ods = dynamic_bitmap_count1(&lflow->dpg_bitmap);
+ ovs_assert(n_ods);
+ if (n_ods == 1) {
/* There is only one datapath, so it should be moved out of the
* group to a single 'od'. */
size_t index = dynamic_bitmap_scan(&lflow->dpg_bitmap, true, 0);
@@ -1180,8 +1174,7 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
sbrec_logical_flow_set_logical_dp_group(sbflow, NULL);
} else {
sbrec_logical_flow_set_logical_datapath(sbflow, NULL);
- lflow->dpg = ovn_dp_group_get(dp_groups, lflow->n_ods,
- lflow->dpg_bitmap.map,
+ lflow->dpg = ovn_dp_group_get(dp_groups, &lflow->dpg_bitmap,
n_datapaths);
if (lflow->dpg) {
/* Update the dpg's sb dp_group. */
@@ -1214,8 +1207,8 @@ 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,
- is_switch, ls_datapaths, lr_datapaths);
+ &lflow->dpg_bitmap, is_switch,
+ ls_datapaths, lr_datapaths);
}
sbrec_logical_flow_set_logical_dp_group(sbflow,
lflow->dpg->dp_group);
@@ -1232,13 +1225,14 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
static struct ovn_dp_group *
ovn_dp_group_find(const struct hmap *dp_groups,
- const unsigned long *dpg_bitmap, size_t bitmap_len,
- uint32_t hash)
+ const struct dynamic_bitmap *dpg_bitmap,
+ size_t bitmap_len, uint32_t hash)
{
struct ovn_dp_group *dpg;
HMAP_FOR_EACH_WITH_HASH (dpg, node, hash, dp_groups) {
- if (bitmap_equal(dpg->bitmap, dpg_bitmap, bitmap_len)) {
+ if (dynamic_bitmap_equal(&dpg->bitmap, dpg_bitmap)) {
+ dynamic_bitmap_realloc(&dpg->bitmap, bitmap_len);
return dpg;
}
}
@@ -1268,7 +1262,7 @@ ovn_dp_group_release(struct hmap *dp_groups, struct
ovn_dp_group *dpg)
static void
ovn_dp_group_destroy(struct ovn_dp_group *dpg)
{
- bitmap_free(dpg->bitmap);
+ dynamic_bitmap_free(&dpg->bitmap);
free(dpg);
}
diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
index 6df44fb21..c1e72d1be 100644
--- a/northd/lflow-mgr.h
+++ b/northd/lflow-mgr.h
@@ -159,7 +159,7 @@ void lflow_table_add_lflow(struct lflow_table *, const
struct ovn_datapath *,
struct sbrec_logical_dp_group;
struct ovn_dp_group {
- unsigned long *bitmap;
+ struct dynamic_bitmap bitmap;
const struct sbrec_logical_dp_group *dp_group;
struct uuid dpg_uuid;
struct hmap_node node;
@@ -174,13 +174,13 @@ ovn_dp_groups_init(struct hmap *dp_groups)
void ovn_dp_groups_clear(struct hmap *dp_groups);
void ovn_dp_groups_destroy(struct hmap *dp_groups);
-struct ovn_dp_group *ovn_dp_group_get(struct hmap *dp_groups, size_t desired_n,
- const unsigned long *desired_bitmap,
- size_t bitmap_len);
+struct ovn_dp_group *ovn_dp_group_get(
+ struct hmap *dp_groups,
+ const struct dynamic_bitmap *desired_bitmap,
+ size_t bitmap_len);
struct ovn_dp_group *ovn_dp_group_create(
struct ovsdb_idl_txn *ovnsb_txn, struct hmap *dp_groups,
struct sbrec_logical_dp_group *sb_group,
- size_t desired_n,
const struct dynamic_bitmap *desired_bitmap,
bool is_switch,
const struct ovn_datapaths *ls_datapaths,
@@ -199,9 +199,9 @@ dec_ovn_dp_group_ref(struct hmap *dp_groups, struct
ovn_dp_group *dpg)
if (!dpg->refcnt) {
hmap_remove(dp_groups, &dpg->node);
- free(dpg->bitmap);
+ dynamic_bitmap_free(&dpg->bitmap);
free(dpg);
}
}
-#endif /* LFLOW_MGR_H */
\ No newline at end of file
+#endif /* LFLOW_MGR_H */
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 448bc66ae..a88b71ca8 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3026,6 +3026,40 @@ OVS_WAIT_UNTIL([grep "all port tunnel ids exhausted"
northd/ovn-northd.log])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([check LS/LR single line configuration])
+ovn_start
+
+cmd=""
+for i in {1..2048..1}; do
+ cmd="${cmd} -- ls-add lsw-${i}"
+ if test $(($i % 100)) -eq 0; then
+ check ovn-nbctl $cmd
+ cmd=""
+ fi
+done
+
+check ovn-nbctl --wait=sb $cmd
+
+check_row_count nb:Logical_Switch 2048
+wait_row_count sb:Datapath_Binding 2048
+
+cmd=""
+for i in {1..2048..1}; do
+ cmd="${cmd} -- lr-add lr-${i}"
+ if test $(($i % 100)) -eq 0; then
+ check ovn-nbctl $cmd
+ cmd=""
+ fi
+done
+
+check ovn-nbctl --wait=sb $cmd
+check_row_count nb:Logical_Router 2048
+wait_row_count sb:Datapath_Binding 4096
+
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD_NO_HV([
AT_SETUP([check VXLAN encap in IC-mode])
ovn_start
--
2.51.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
