[ovs-dev] [PATCH] lib/dpif: Fix crash in format_odp_actions, actions = 0x0

[Madhu Challa]

Forgot to attach the backtrace ::

(gdb) bt
#0  nl_attr_is_valid (maxlen=0xcfea1104, nla=0x0) at lib/netlink.h:139
#1  format_odp_actions (ds=0x7f77c7ffc800, actions=0x0,
    actions_len=0x7f77cfea1104) at lib/odp-util.c:608
#2  0x0000000000456749 in log_flow_message (error=0x2,
    operation=0x506548 "flow_get", key=0x7f779405ac30, key_len=0x60,
    mask=0x7f77ac000ad0, mask_len=0x78f1e0, stats=0x7f77c7ffeb50,
actions=0x0,
    actions_len=0x7f77cfea1104, dpif=<optimized out>) at lib/dpif.c:1498
#3  0x0000000000456de8 in log_flow_get_message (error=0x2,
get=0x7f77c7ffc9a8,
    dpif=0x19178f0) at lib/dpif.c:1588
#4  dpif_operate (dpif=0x19178f0, ops=0x7f77c7ffc9f8, n_ops=0x1)
    at lib/dpif.c:1158
#5  0x00000000004572ce in dpif_flow_get (dpif=<optimized out>,
    key=<optimized out>, key_len=<optimized out>, buf=<optimized out>,
    flow=<optimized out>) at lib/dpif.c:852
#6  0x0000000000431514 in handle_missed_revalidation (ukey=0x7f779405aba0,
    revalidator=<optimized out>) at ofproto/ofproto-dpif-upcall.c:1609
#7  revalidator_sweep__ (revalidator=0x19592f0, purge=0x0)
    at ofproto/ofproto-dpif-upcall.c:1636
#8  0x0000000000431da7 in revalidator_sweep (revalidator=0x19592f0)
    at ofproto/ofproto-dpif-upcall.c:1657
#9  udpif_revalidator (arg=0x19592f0) at ofproto/ofproto-dpif-upcall.c:705
#10 0x000000000049e652 in ovsthread_wrapper (aux_=<optimized out>)
    at lib/ovs-thread.c:338
#11 0x00007f77cfe9de9a in start_thread (arg=0x7f77c7fff700)
    at pthread_create.c:308
#12 0x00007f77cf6c63fd in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#13 0x0000000000000000 in ?? ()

On Fri, Oct 17, 2014 at 4:25 PM, Madhu Challa <challa at noironetworks.com>
wrote:

> You are correct Ben. I confused with the put that has actions in
> dpif_flow_put. I guess I was not able to repro the issue then. Let me repro
> it and I will resend the fix.
>
> Thanks.
>
> On Fri, Oct 17, 2014 at 4:17 PM, Ben Pfaff <blp at nicira.com> wrote:
>
>> On Fri, Oct 17, 2014 at 04:07:25PM -0700, Madhu Challa wrote:
>> > dpif_flow_get initializes the flow_get part of the union, down the stack
>> > log_flow_message checks for actions || actions_len that could contain
>> > garbage leading to the crash.
>> >
>> > saw the crash once when running stress tests. can be easily recreated
>> > by running ovs-dpctl del-flows in a loop when traffic is going on
>> >
>> > Signed-off-by: Madhu Challa <challa at noironetworks.com>
>>
>> The actions aren't in the dpif_op so I don't see how this would help.
>> Can you explain?
>>
>> The actions are, instead, in the caller-provided dpif_flow.  I guess
>> that the error is here in dpif_operate() where the code clears the
>> flow only after trying to log uninitialized garbage from it:
>>                     log_flow_get_message(dpif, get, error);
>>
>>                     if (error) {
>>                         memset(get->flow, 0, sizeof *get->flow);
>>                     }
>>
>
>