On Mon, Apr 5, 2021 at 1:32 AM Ilya Maximets <i.maxim...@ovn.org> wrote:
>
> 'dpif_execute_helper_cb' doesn't initilalize the 'hash' field that
> may be passed down to datapath and might cause execution of a different
> set of actions, e.g. on recirculation.
>
> Thread 6 handler27:
> Conditional jump or move depends on uninitialised value(s)
> at 0x53A2C2: dpif_netlink_encode_execute (dpif-netlink.c:1841)
> by 0x53A2C2: dpif_netlink_operate__ (dpif-netlink.c:1919)
> by 0x53A82D: dpif_netlink_operate_chunks (dpif-netlink.c:2238)
> by 0x53A82D: dpif_netlink_operate (dpif-netlink.c:2297)
> by 0x48135F: dpif_operate (dpif.c:1366)
> by 0x481923: dpif_execute.part.24 (dpif.c:1320)
> by 0x481C46: dpif_execute (dpif.c:1312)
> by 0x481C46: dpif_execute_helper_cb (dpif.c:1243)
> by 0x4AE943: odp_execute_actions (odp-execute.c:865)
> by 0x47F272: dpif_execute_with_help (dpif.c:1296)
> by 0x4812FF: dpif_operate (dpif.c:1422)
> by 0x442226: handle_upcalls (ofproto-dpif-upcall.c:1617)
> by 0x442226: recv_upcalls.isra.36 (ofproto-dpif-upcall.c:855)
> by 0x442351: udpif_upcall_handler (ofproto-dpif-upcall.c:755)
> by 0x4FDE2C: ovsthread_wrapper (ovs-thread.c:383)
> by 0x5E19159: start_thread (in /usr/lib64/libpthread-2.28.so)
> by 0x69ECF72: clone (in /usr/lib64/libc-2.28.so)
> Uninitialised value was created by a stack allocation
> at 0x481966: dpif_execute_helper_cb (dpif.c:1159)
>
> Additionally added a missing comment to the 'struct dpif_execute'.
Thanks Ilya
Acked-by: Tonghao Zhang <xiangxia.m....@gmail.com>
> CC: Tonghao Zhang <xiangxia.m....@gmail.com>
> Fixes: 0442bfb11d6c ("ofproto-dpif-upcall: Echo HASH attribute back to
> datapath.")
> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
> ---
> lib/dpif.c | 1 +
> lib/dpif.h | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 56d0b4a65..26e8bfb7d 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1240,6 +1240,7 @@ dpif_execute_helper_cb(void *aux_, struct
> dp_packet_batch *packets_,
> execute.needs_help = false;
> execute.probe = false;
> execute.mtu = 0;
> + execute.hash = 0;
> aux->error = dpif_execute(aux->dpif, &execute);
> log_execute_message(aux->dpif, &this_module, &execute,
> true, aux->error);
> diff --git a/lib/dpif.h b/lib/dpif.h
> index ecda896c7..f9728e673 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -727,7 +727,7 @@ struct dpif_execute {
> bool probe; /* Suppress error messages. */
> unsigned int mtu; /* Maximum transmission unit to fragment.
> 0 if not a fragmented packet */
> - uint64_t hash;
> + uint64_t hash; /* Packet flow hash. 0 if not specified.
> */
> const struct flow *flow; /* Flow extracted from 'packet'. */
>
> /* Input, but possibly modified as a side effect of execution. */
> --
> 2.26.2
>
--
Best regards, Tonghao
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
