Hi Yifeng,
Thanks for the patch, I can reproduce the issue using
$ make check-system-userspace TESTSUITEFLAGS='-k resume'
ASAN reports
==127707==ERROR: AddressSanitizer: heap-use-after-free on address
0x61f000020690 at pc 0x00000089cecf bp 0x7fff38f95690 sp
0x7fff38f95688
READ of size 4 at 0x61f000020690 thread T0
#0 0x89cece in tun_metadata_get_fmd /root/ovs/lib/tun-metadata.c:394:52
#1 0x66a3f9 in flow_get_metadata /root/ovs/lib/flow.c:1236:5
#2 0x58a9ca in process_upcall
/root/ovs/ofproto/ofproto-dpif-upcall.c:1538:13
#3 0x57a723 in upcall_cb /root/ovs/ofproto/ofproto-dpif-upcall.c:1311:13
However, applying your fix (patch 1/2) and run
$ make check-system-userspace TESTSUITEFLAGS='-k resume'
fix the crash but trigger other fail.
Can you take a look? Some comments below:
On Thu, Mar 26, 2020 at 12:58 PM Yifeng Sun <[email protected]> wrote:
>
> This patch enhances a system traffic test to prevent regression on
> the tunnel metadata table (tun_table) handling with frozen state.
> Without a proper fix this test can crash ovs-vswitchd due to a
> use-after-free bug on tun_table.
>
> These are the timed sequence of how this bug is triggered:
>
> - Adds an OpenFlow rule in OVS that matches Geneve tunnel metadata that
> contains a controller action.
> - When the first packet matches the aforementioned OpenFlow rule,
> during the miss upcall, OVS stores a pointer to the tun_table (that
> decodes the Geneve tunnel metadata) in a frozen state and pushes down
> a datapath flow into kernel datapath.
> - Issues a add-tlv-map command to reprogram the tun_table on OVS.
> OVS frees the old tun_table and create a new tun_table.
> - A subsequent packet hits the kernel datapath flow again. Since
> there is a controller action associated with that flow, it triggers
> slow path controller upcall.
> - In the slow path controller upcall, OVS derives the tun_table
> from the frozen state, which points to the old tun_table that is
> already being freed at this time point.
> - In order to access the tunnel metadata, OVS uses the invalid
> pointer that points to the old tun_table and triggers the core dump.
>
> Signed-off-by: Yi-Hung Wei <[email protected]>
> Signed-off-by: Yifeng Sun <[email protected]>
> Co-authored-by: Yi-Hung Wei <[email protected]>
> ---
> tests/system-traffic.at | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 4a39c929c207..992de8546c41 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -611,6 +611,20 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 10.1.1.100 |
> FORMAT_PING], [0], [dnl
> 3 packets transmitted, 3 received, 0% packet loss, time 0ms
> ])
>
> +dnl Test OVS handles TLV map modifictions properly when restores frozen
> state.
> +NS_CHECK_EXEC([at_ns0], [ping 10.1.1.100 > ping.out &])
since ping.out is not checked, can you use ping -c <count> -q?
> +
> +sleep 2
do we need this sleep 2
> +
> +AT_CHECK([ovs-ofctl add-tlv-map br0
> "{class=0xffff,type=0x88,len=4}->tun_metadata1"])
> +sleep 1
> +AT_CHECK([ovs-ofctl add-tlv-map br0
> "{class=0xffff,type=0x99,len=4}->tun_metadata2"])
> +sleep 1
> +AT_CHECK([ovs-ofctl add-tlv-map br0
> "{class=0xffff,type=0xaa,len=4}->tun_metadata3"])
> +sleep 1
> +
> +dnl At this point, ovs-vswitchd will either crash or everything is OK.
can you remove this comment?
>` +
> OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
