On 4/23/23 05:47, wangyunjian wrote: > Is there any issue with this patch? Why is it in the Superseded state?
Hi. Not sure what happened here. I moved it back to 'New' for now. Best regards, Ilya Maximets. > >> -----Original Message----- >> From: wangyunjian >> Sent: Wednesday, April 19, 2023 2:29 PM >> To: d...@openvswitch.org; i.maxim...@ovn.org >> Cc: luyicai <luyi...@huawei.com>; wangyunjian <wangyunj...@huawei.com> >> Subject: [ovs-dev] [PATCH] ofproto-dpif-xlate: Fix use-after-free when >> xlate_actions(). >> >> Currently, bundle->cvlans and xbundle->cvlans are pointing to the same >> memory location. This can cause issues if the main thread modifies >> bundle->cvlans and frees it while the revalidator thread is still accessing >> xbundle->cvlans. This can result in use after free errors. >> >> AddressSanitizer: heap-use-after-free on address 0x615000007b08 at pc >> 0x0000004ede1e bp 0x7f3120ee0310 sp 0x7f3120ee0300 READ of size 8 at >> 0x615000007b08 thread T25 (revalidator25) >> #0 0x4ede1d in bitmap_is_set lib/bitmap.h:91 >> #1 0x4fcb26 in xbundle_allows_cvlan ofproto/ofproto-dpif-xlate.c:2028 >> #2 0x4fe279 in input_vid_is_valid ofproto/ofproto-dpif-xlate.c:2294 >> #3 0x502abf in xlate_normal ofproto/ofproto-dpif-xlate.c:3051 >> #4 0x5164dc in xlate_output_action ofproto/ofproto-dpif-xlate.c:5361 >> #5 0x522576 in do_xlate_actions ofproto/ofproto-dpif-xlate.c:7047 >> #6 0x52a751 in xlate_actions ofproto/ofproto-dpif-xlate.c:8061 >> #7 0x4e2b66 in xlate_key ofproto/ofproto-dpif-upcall.c:2212 >> #8 0x4e2e13 in xlate_ukey ofproto/ofproto-dpif-upcall.c:2227 >> #9 0x4e345d in revalidate_ukey__ ofproto/ofproto-dpif-upcall.c:2276 >> #10 0x4e3f85 in revalidate_ukey ofproto/ofproto-dpif-upcall.c:2395 >> #11 0x4e7ac5 in revalidate ofproto/ofproto-dpif-upcall.c:2858 >> #12 0x4d9ed3 in udpif_revalidator ofproto/ofproto-dpif-upcall.c:1010 >> #13 0x7cd92e in ovsthread_wrapper lib/ovs-thread.c:423 >> #14 0x7f312ff01f3a (/usr/lib64/libpthread.so.0+0x8f3a) >> #15 0x7f312fc8f51f in clone (/usr/lib64/libc.so.6+0xf851f) >> >> 0x615000007b08 is located 8 bytes inside of 512-byte region >> [0x615000007b00,0x615000007d00) freed by thread T0 here: >> #0 0x7f3130378ad8 in free (/usr/lib64/libasan.so.4+0xe0ad8) >> #1 0x49044e in bundle_set ofproto/ofproto-dpif.c:3431 >> #2 0x444f92 in ofproto_bundle_register ofproto/ofproto.c:1455 >> #3 0x40e6c9 in port_configure vswitchd/bridge.c:1300 >> #4 0x40bcfd in bridge_reconfigure vswitchd/bridge.c:921 >> #5 0x41f1a9 in bridge_run vswitchd/bridge.c:3313 >> #6 0x42d4fb in main vswitchd/ovs-vswitchd.c:132 >> #7 0x7f312fbbcc86 in __libc_start_main (/usr/lib64/libc.so.6+0x25c86) >> >> previously allocated by thread T0 here: >> #0 0x7f3130378e70 in __interceptor_malloc >> (/usr/lib64/libasan.so.4+0xe0e70) >> #1 0x8757fe in xmalloc__ lib/util.c:140 >> #2 0x8758da in xmalloc lib/util.c:175 >> #3 0x875927 in xmemdup lib/util.c:188 >> #4 0x475f63 in bitmap_clone lib/bitmap.h:79 >> #5 0x47797c in vlan_bitmap_clone lib/vlan-bitmap.h:40 >> #6 0x49048d in bundle_set ofproto/ofproto-dpif.c:3433 >> #7 0x444f92 in ofproto_bundle_register ofproto/ofproto.c:1455 >> #8 0x40e6c9 in port_configure vswitchd/bridge.c:1300 >> #9 0x40bcfd in bridge_reconfigure vswitchd/bridge.c:921 >> #10 0x41f1a9 in bridge_run vswitchd/bridge.c:3313 >> #11 0x42d4fb in main vswitchd/ovs-vswitchd.c:132 >> #12 0x7f312fbbcc86 in __libc_start_main (/usr/lib64/libc.so.6+0x25c86) >> >> Fixes: fed8962aff57 ("Add new port VLAN mode "dot1q-tunnel"") >> Signed-off-by: Yunjian Wang <wangyunj...@huawei.com> >> --- >> ofproto/ofproto-dpif-xlate.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c >> index >> c01177718..455ca3dac 100644 >> --- a/ofproto/ofproto-dpif-xlate.c >> +++ b/ofproto/ofproto-dpif-xlate.c >> @@ -66,6 +66,7 @@ >> #include "tunnel.h" >> #include "util.h" >> #include "uuid.h" >> +#include "vlan-bitmap.h" >> >> COVERAGE_DEFINE(xlate_actions); >> COVERAGE_DEFINE(xlate_actions_oversize); >> @@ -1028,7 +1029,12 @@ xlate_xbundle_set(struct xbundle *xbundle, >> xbundle->qinq_ethtype = qinq_ethtype; >> xbundle->vlan = vlan; >> xbundle->trunks = trunks; >> - xbundle->cvlans = cvlans; >> + if (xbundle->cvlans == NULL) { >> + xbundle->cvlans = vlan_bitmap_clone(cvlans); >> + } else if (!vlan_bitmap_equal(xbundle->cvlans, cvlans)) { >> + ovsrcu_postpone(free, xbundle->cvlans); >> + xbundle->cvlans = vlan_bitmap_clone(cvlans); >> + } >> xbundle->use_priority_tags = use_priority_tags; >> xbundle->floodable = floodable; >> xbundle->protected = protected; >> @@ -1380,6 +1386,7 @@ xlate_xbundle_remove(struct xlate_cfg *xcfg, struct >> xbundle *xbundle) >> ovs_list_remove(&xbundle->list_node); >> bond_unref(xbundle->bond); >> lacp_unref(xbundle->lacp); >> + free(xbundle->cvlans); >> free(xbundle->name); >> free(xbundle); >> } >> -- >> 2.27.0 > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev