Ilya Maximets <[email protected]> writes: > The commit in the Fixes tag below made so flow->mask free is scheduled > via RCU right after it is removed from the flow table. The pointer > stays in the flow structure and it can be accessible while in the same > RCU critical section. This is done to avoid requiring ovs_mutex for > the ovs_flow_free(). > > However, while removing the flow during processing of CMD_DEL, we do > not take RCU read lock before the removal, and ovs_flow_cmd_fill_info() > uses the flow->mask pointer afterwards. The RCU read lock is taken, > but it's already late at that point. The comment on that line > acknowledges that the lock is cosmetic and doesn't serve a real purpose. > > This leads to use-after-free if the RCU grace period passes between > removal and the filling. It is a short race window, but it is there > and can lead to a real crash in case memory allocation for the info > takes a bit longer: > > BUG: KASAN: slab-use-after-free in __ovs_nla_put_key > net/openvswitch/flow_netlink.c:1996 > BUG: KASAN: slab-use-after-free in ovs_nla_put_key+0x2463/0x2e30 > net/openvswitch/flow_netlink.c:2250 > Read of size 4 at addr ffff88801ee89970 by task ovs_flow_del_ec/9487 > > Call Trace: > <TASK> > __ovs_nla_put_key net/openvswitch/flow_netlink.c:1996 > ovs_nla_put_key+0x2463/0x2e30 net/openvswitch/flow_netlink.c:2250 > ovs_flow_cmd_fill_info+0x420/0x9c0 net/openvswitch/datapath.c:930 > ovs_flow_cmd_del+0x53a/0x970 net/openvswitch/datapath.c:1467 > ... > netlink_rcv_skb+0x156/0x420 net/netlink/af_netlink.c:2556 > </TASK> > > Allocated by task 9487: > mask_alloc net/openvswitch/flow_table.c:967 > flow_mask_insert net/openvswitch/flow_table.c:1012 > ovs_flow_tbl_insert+0xea2/0x1a90 net/openvswitch/flow_table.c:1084 > ovs_flow_cmd_new+0x7e3/0xd90 net/openvswitch/datapath.c:1086 > ... > netlink_rcv_skb+0x156/0x420 net/netlink/af_netlink.c:2556 > > Freed by task 9485: > rcu_free_sheaf+0x1e/0x100 mm/slub.c:5978 > rcu_do_batch kernel/rcu/tree.c:2645 > rcu_core+0x59c/0x10c0 kernel/rcu/tree.c:2897 > handle_softirqs+0x1e4/0x9a0 kernel/softirq.c:622 > ... > instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1062 > > ovs_flow_tbl_remove() must be called after the ovs_flow_cmd_fill_info() > to avoid this race. This also helps with cleaning up the forced cast > and the cosmetic RCU read lock. Before the commit in the Fixes tag the > order did not matter as long as the flow object itself was not freed. > > A wider RCU critical section could be another option, but we have a > GFP_KERNEL allocation in the way. > > Reported by Trend Micro's Zero Day Initiative as ZDI-CAN-32042. > > Fixes: 56c19868e115 ("openvswitch: Make flow mask removal symmetric.") > Cc: [email protected] > Signed-off-by: Ilya Maximets <[email protected]> > ---
Reviewed-by: Aaron Conole <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
