On Tue, Mar 8, 2022 at 8:44 PM Ilya Maximets <[email protected]> wrote:
>
> On 3/8/22 17:39, David Marchand wrote:
> > On Mon, Mar 7, 2022 at 11:28 PM Ilya Maximets <[email protected]> wrote:
> >>
> >> 'ofproto' should still be in place while freeing meter id, but this is
> >> not ensured in any way leading to the use-after-free:
> >>
> >> ==13702==ERROR: AddressSanitizer: heap-use-after-free
> >> READ of size 8 at 0x614000000c50 thread T3 (urcu2)
> >> 0 0x536656 in free_meter_id ofproto/ofproto-dpif.c:6780:37
> >> 1 0x7356d0 in ovsrcu_call_postponed lib/ovs-rcu.c:346:13
> >> 2 0x735b21 in ovsrcu_postpone_thread lib/ovs-rcu.c:362:14
> >> 3 0x73a30c in ovsthread_wrapper lib/ovs-thread.c:422:12
> >> 4 0x7f7b1a7876da in start_thread
> >> 5 0x7f7b19d0671e in clone
> >>
> >> 0x614000000c50 is located 16 bytes inside of 400-byte region
> >> freed by thread T0 here:
> >> 0 0x49640d in free (vswitchd/ovs-vswitchd+0x49640d)
> >> 1 0x517e38 in destruct ofproto/ofproto-dpif.c:1851:5
> >> 2 0x4f0bb0 in ofproto_destroy ofproto/ofproto.c:1773:5
> >> 3 0x4c71e4 in bridge_destroy vswitchd/bridge.c:3608:9
> >> 4 0x4c6f4a in bridge_exit vswitchd/bridge.c:553:9
> >> 5 0x4e109a in main vswitchd/ovs-vswitchd.c:146:5
> >> 6 0x7f7b19c06bf6 in __libc_start_main
> >>
> >> Since introduction of the ofproto refcount this issue can be fixed
> >> by taking the 'ofproto' reference before postponing the operation.
> >> This will guarantee that 'ofproto' and 'baker' are not destroyed until
> >> all ids are freed.
> >
> > We prevent ofproto from getting shot thanks to refcount, but I don't
> > understand what prevents backer content from being uninitialised (like
> > destroying the meter id pool) in close_dpif_backer().
>
> Successfully opened backer is closed only as part of the destruction
> of the last ofproto instance that uses it. So, if we're holding the
Yes, I think that's what happens here.
We are closing the last ofproto instance: backer gets shot in
close_dpif_backer()/destroy().
And there is a rcu postponed free_meter_id() with reference to ofproto->backer.
> ofproto reference, there is at least one ofproto that is using that
> backer, so it will not be closed. Am I missing something?
Before and after patch show the same in my env (I had to wait a bit to
reproduce):
Without patch:
# while make check -C build-asan TESTSUITEFLAGS="-k meter -d"; do true; done
...
1212: ofproto-dpif - patch ports - meter (clone) FAILED (ovs-macros.at:217)
...
ERROR: AddressSanitizer: heap-use-after-free on address 0x614000000e50
at pc 0x000000f0c0fc bp 0x7faf54cae7c0 sp 0x7faf54cae7b0
READ of size 8 at 0x614000000e50 thread T4 (urcu3)
#0 0xf0c0fb in free_meter_id ../ofproto/ofproto-dpif.c:6780
#1 0x118d6c9 in ovsrcu_call_postponed ../lib/ovs-rcu.c:346
#2 0x118dad2 in ovsrcu_postpone_thread ../lib/ovs-rcu.c:362
#3 0x11938cb in ovsthread_wrapper ../lib/ovs-thread.c:422
#4 0x7faf5bd25179 in start_thread
/usr/src/debug/glibc-2.28/nptl/pthread_create.c:479
#5 0x7faf594f7dc2 in __clone (/lib64/libc.so.6+0xfcdc2)
0x614000000e50 is located 16 bytes inside of 400-byte region
[0x614000000e40,0x614000000fd0)
freed by thread T0 here:
#0 0x7faf5c2347e0 in __interceptor_free (/lib64/libasan.so.5+0xef7e0)
#1 0xf18268 in close_dpif_backer ../ofproto/ofproto-dpif.c:714
#2 0xf18d08 in destruct ../ofproto/ofproto-dpif.c:1851
#3 0xf000e9 in ofproto_destroy ../ofproto/ofproto.c:1773
#4 0xeb2487 in bridge_destroy ../vswitchd/bridge.c:3608
#5 0xecac08 in bridge_exit ../vswitchd/bridge.c:553
#6 0xed1624 in main ../vswitchd/ovs-vswitchd.c:146
#7 0x7faf5941e492 in __libc_start_main ../csu/libc-start.c:314
previously allocated by thread T0 here:
#0 0x7faf5c234ba8 in __interceptor_malloc (/lib64/libasan.so.5+0xefba8)
#1 0x121a732 in xmalloc__ ../lib/util.c:137
#2 0x121a755 in xmalloc ../lib/util.c:172
#3 0xf351bc in open_dpif_backer ../ofproto/ofproto-dpif.c:770
#4 0xf351bc in construct ../ofproto/ofproto-dpif.c:1668
#5 0xeedf86 in ofproto_create ../ofproto/ofproto.c:554
#6 0xec2080 in bridge_reconfigure ../vswitchd/bridge.c:882
#7 0xecfe01 in bridge_run ../vswitchd/bridge.c:3331
#8 0xed153a in main ../vswitchd/ovs-vswitchd.c:129
#9 0x7faf5941e492 in __libc_start_main ../csu/libc-start.c:314
With patch:
ERROR: AddressSanitizer: heap-use-after-free on address 0x614000000e50
at pc 0x000000f0c179 bp 0x7f6ee58097c0 sp 0x7f6ee58097b0
READ of size 8 at 0x614000000e50 thread T4 (urcu3)
#0 0xf0c178 in free_meter_id ../ofproto/ofproto-dpif.c:6780
#1 0x118d6f0 in ovsrcu_call_postponed ../lib/ovs-rcu.c:346
#2 0x118daf9 in ovsrcu_postpone_thread ../lib/ovs-rcu.c:362
#3 0x11938f2 in ovsthread_wrapper ../lib/ovs-thread.c:422
#4 0x7f6eec880179 in start_thread
/usr/src/debug/glibc-2.28/nptl/pthread_create.c:479
#5 0x7f6eea052dc2 in __clone (/lib64/libc.so.6+0xfcdc2)
0x614000000e50 is located 16 bytes inside of 400-byte region
[0x614000000e40,0x614000000fd0)
freed by thread T0 here:
#0 0x7f6eecd8f7e0 in __interceptor_free (/lib64/libasan.so.5+0xef7e0)
#1 0xf1828f in close_dpif_backer ../ofproto/ofproto-dpif.c:714
#2 0xf18d2f in destruct ../ofproto/ofproto-dpif.c:1851
#3 0xf000e9 in ofproto_destroy ../ofproto/ofproto.c:1773
#4 0xeb2487 in bridge_destroy ../vswitchd/bridge.c:3608
#5 0xecac08 in bridge_exit ../vswitchd/bridge.c:553
#6 0xed1624 in main ../vswitchd/ovs-vswitchd.c:146
#7 0x7f6ee9f79492 in __libc_start_main ../csu/libc-start.c:314
previously allocated by thread T0 here:
#0 0x7f6eecd8fba8 in __interceptor_malloc (/lib64/libasan.so.5+0xefba8)
#1 0x121a759 in xmalloc__ ../lib/util.c:137
#2 0x121a77c in xmalloc ../lib/util.c:172
#3 0xf351e3 in open_dpif_backer ../ofproto/ofproto-dpif.c:770
#4 0xf351e3 in construct ../ofproto/ofproto-dpif.c:1668
#5 0xeedf86 in ofproto_create ../ofproto/ofproto.c:554
#6 0xec2080 in bridge_reconfigure ../vswitchd/bridge.c:882
#7 0xecfe01 in bridge_run ../vswitchd/bridge.c:3331
#8 0xed153a in main ../vswitchd/ovs-vswitchd.c:129
#9 0x7f6ee9f79492 in __libc_start_main ../csu/libc-start.c:314
--
David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
