[ovs-dev] 答复: [ovs-discuss] a question about ovs crash relationship with learn action

[wangyunjian]

This patch has problem.
But when the cache has XC_LEARN entry，the cache can't be executed multiple times
in xlate_push_stats. The ofm's match->flow in XC_LEARN entry may use-after-free.

>-----邮件原件-----
>发件人: Ben Pfaff [mailto:blp at ovn.org]
>发送时间: 2018年7月11日 5:00
>收件人: wangyunjian <wangyunjian at huawei.com>
>抄送: dev at openvswitch.org; ovs-discuss at openvswitch.org; Lilijun (Jerry, Cloud
>Networking) <jerry.lilijun at huawei.com>
>主题: Re: [ovs-discuss] a question about ovs crash relationship with learn action
>
>Can you explain further?  Throwing away the cache entries makes the cache
>less useful.
>
>Are you really using v2.7.0?  None of the line numbers match up, either in the
>backtrace or in the patch.
>
>On Tue, Jun 26, 2018 at 10:38:50AM +0000, wangyunjian wrote:
>> I think the function xlate_cache_clear() needs be called after
>> xlate_push_stats() to avoid xcache->entries use-after-free.
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c
>> b/ofproto/ofproto-dpif-upcall.c index 85f5792..71b846c 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -2252,6 +2252,7 @@ revalidate_ukey(struct udpif *udpif, struct
>udpif_key *ukey,
>>      /* Stats for deleted flows will be attributed upon flow deletion. Skip. */
>>      if (result != UKEY_DELETE) {
>>          xlate_push_stats(ukey->xcache, &push);
>> +        xlate_cache_clear(ukey->xcache);
>>          ukey->stats = *stats;
>>          ukey->reval_seq = reval_seq;
>>      }
>>
>> > From: wangyunjian
>> > Sent: Monday, June 25, 2018 2:19 PM
>> > To: ovs-discuss at openvswitch.org
>> > Cc: 'blp at ovn.org' <blp at ovn.org>; Lilijun (Jerry, Cloud Networking)
>> > <jerry.lilijun at huawei.com>
>> > Subject: [ovs-discuss] a question about ovs crash relationship with
>> > learn action
>> >
>> > I'm running OVS 2.7.0 on a Linux 3.10.0 kernel. I found a ovs crash.
>> > I doubt it's caused by use-after-free set match->flow = NULL in
>> > minimatch_destroy function with following stack:
>> >
>> > (gdb) bt
>> > #0  0x00007ff273b71197 in raise () from /usr/lib64/libc.so.6
>> > #1  0x00007ff273b72888 in abort () from /usr/lib64/libc.so.6
>> > #2  0x0000000000787289 in PAT_abort ()
>> > #3  0x00000000007843cd in patchIllInsHandler ()
>> > #4  <signal handler called>
>> > #5  0x00000000004cbfae in miniflow_n_values (flow=0x0) at
>> > lib/flow.h:540
>> > #6  0x00000000004cc95f in minimask_hash (mask=0x0, basis=0) at
>> > lib/classifier-private.h:321
>> > #7  0x00000000004cf613 in find_subtable (cls=0x38ad6e8, mask=0x0) at
>> > lib/classifier.c:1406
>> > #8  0x00000000004cefa7 in classifier_find_rule_exactly
>> > (cls=0x38ad6e8, target=0x7ff118025500, version=18446744073709551615)
>> > at lib/classifier.c:1178
>> > #9  0x000000000047bcaf in collect_rules_strict (ofproto=0x389bc30,
>> > criteria=0x7ff1180254f8, rules=0x7ff118025588) at
>> > ofproto/ofproto.c:4253
>> > #10 0x000000000047eba3 in modify_flow_start_strict
>> > (ofproto=0x389bc30, ofm=0x7ff1180254f0) at ofproto/ofproto.c:5492
>> > #11 0x0000000000482c9f in ofproto_flow_mod_start (ofproto=0x389bc30,
>> > ofm=0x7ff1180254f0) at ofproto/ofproto.c:7506
>> > #12 0x000000000047dc01 in ofproto_flow_mod_learn_start
>> > (ofm=0x7ff1180254f0) at ofproto/ofproto.c:5088
>> > #13 0x000000000047dd4b in ofproto_flow_mod_learn
>> > (ofm=0x7ff1180254f0, keep_ref=true) at ofproto/ofproto.c:5140
>> > #14 0x00000000004b55d4 in xlate_push_stats_entry
>> > (entry=0x7ff118015148, stats=0x7ff11d6675f0) at
>> > ofproto/ofproto-dpif-xlate-cache.c:130
>> > #15 0x00000000004b57b6 in xlate_push_stats (xcache=0x7ff1180254a0,
>> > stats=0x7ff11d6675f0) at ofproto/ofproto-dpif-xlate-cache.c:183
>> > #16 0x00000000004a312f in revalidate_ukey (udpif=0x38a5260,
>> > ukey=0x7ff0fc015910, stats=0x7ff11d668260,
>> > odp_actions=0x7ff11d66a3d0, reval_seq=25145760,
>> > recircs=0x7ff11d66a3b0) at ofproto/ofproto-dpif-upcall.c:2134
>> > #17 0x00000000004a3d76 in revalidate (revalidator=0x4cdda08) at
>> > ofproto/ofproto-dpif-upcall.c:2428
>> > #18 0x00000000004a0528 in udpif_revalidator (arg=0x4cdda08) at
>> > ofproto/ofproto-dpif-upcall.c:954
>> > #19 0x000000000058f811 in ovsthread_wrapper (aux_=0x55088a0) at
>> > lib/ovs-thread.c:682
>> > #20 0x00007ff27549adc5 in start_thread () from
>> > /usr/lib64/libpthread.so.0
>> >
>> > Any idea about this?
>> > Thanks,
>> > Yunjian
>>