'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.
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-February/380582.html
Signed-off-by: Ilya Maximets <[email protected]>
---
ofproto/ofproto-dpif.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index a4c44052d..355300b1a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6779,6 +6779,7 @@ free_meter_id(struct free_meter_id_args *args)
dpif_meter_del(ofproto->backer->dpif, args->meter_id, NULL, 0);
id_pool_free_id(ofproto->backer->meter_ids, args->meter_id.uint32);
+ ofproto_unref(&ofproto->up);
free(args);
}
@@ -6794,6 +6795,7 @@ meter_del(struct ofproto *ofproto_, ofproto_meter_id
meter_id)
* especially by the handler and revalidator threads.
* Postpone meter deletion after RCU grace period, so that ongoing
* upcall translation or flow revalidation can complete. */
+ ofproto_ref(ofproto_);
arg->ofproto = ofproto_dpif_cast(ofproto_);
arg->meter_id = meter_id;
ovsrcu_postpone(free_meter_id, arg);
--
2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
