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 leads to use-after-free
error.

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
    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 <wangyunjian@huawei.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 6 insertions, 1 deletions

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index c01177718..29f4daa63 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,10 @@ xlate_xbundle_set(struct xbundle *xbundle,
     xbundle->qinq_ethtype = qinq_ethtype;
     xbundle->vlan = vlan;
     xbundle->trunks = trunks;
-    xbundle->cvlans = cvlans;
+    if (!vlan_bitmap_equal(xbundle->cvlans, cvlans)) {
+        free(xbundle->cvlans);
+        xbundle->cvlans = vlan_bitmap_clone(cvlans);
+    }
     xbundle->use_priority_tags = use_priority_tags;
     xbundle->floodable = floodable;
     xbundle->protected = protected;
@@ -1380,6 +1384,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);
 }