Thanks Lorenzo, I think that ovn_dp_group() functions should take dynamic bitmaps as arguments instead of a static bitmap and a size. This way we ensure we always use the correct capacity for the bitmap.
If you did this, I think there would be no need for the dynamic_bitmap_clone_from_bitmap() function as well. On Wed, Nov 12, 2025 at 10:02 AM Lorenzo Bianconi via dev <[email protected]> wrote: > > 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 | 8 ++++++++ > northd/lflow-mgr.c | 8 +++++--- > northd/lflow-mgr.h | 6 +++--- > tests/ovn-northd.at | 34 ++++++++++++++++++++++++++++++++++ > 4 files changed, 50 insertions(+), 6 deletions(-) > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index aa1a878bb..eda2bd4a0 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -586,6 +586,14 @@ dynamic_bitmap_clone_map(struct dynamic_bitmap *db) > return bitmap_clone(db->map, db->capacity); > } > > +static inline void > +dynamic_bitmap_clone_from_bitmap(struct dynamic_bitmap *db, > + const unsigned long *bitmap, size_t n_bits) > +{ > + db->map = bitmap_clone(bitmap, n_bits); > + db->capacity = n_bits; > +} > + > static inline size_t > dynamic_bitmap_scan(struct dynamic_bitmap *dp, bool target, size_t start) > { > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c > index 3af36c7fd..1a79d1ce9 100644 > --- a/northd/lflow-mgr.c > +++ b/northd/lflow-mgr.c > @@ -836,7 +836,7 @@ ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn, > bitmap_free(dpg_bitmap); > > dpg = xzalloc(sizeof *dpg); > - dpg->bitmap = bitmap_clone(desired_bitmap, bitmap_len); > + dynamic_bitmap_clone_from_bitmap(&dpg->bitmap, desired_bitmap, > bitmap_len); > if (!update_dp_group) { > dpg->dp_group = sb_group; > } else { > @@ -1239,7 +1239,9 @@ ovn_dp_group_find(const struct hmap *dp_groups, > 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 (bitmap_equal(dpg->bitmap.map, dpg_bitmap, > + MIN(bitmap_len, dpg->bitmap.capacity))) { > + dynamic_bitmap_realloc(&dpg->bitmap, bitmap_len); > return dpg; > } > } > @@ -1269,7 +1271,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 91708c8a9..2840e3228 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; > @@ -198,9 +198,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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
