On Wed, Sep 11, 2019 at 02:18:36PM -0700, Yifeng Sun wrote: > Valgrind reported: > > 1305: ofproto-dpif - conntrack - ipv6 > > ==26942== Conditional jump or move depends on uninitialised value(s) > ==26942== at 0x587C00: check_orig_tuple (conntrack.c:1006) > ==26942== by 0x587C00: process_one (conntrack.c:1141) > ==26942== by 0x587C00: conntrack_execute (conntrack.c:1220) > ==26942== by 0x47B00F: dp_execute_cb (dpif-netdev.c:7305) > ==26942== by 0x4AF756: odp_execute_actions (odp-execute.c:794) > ==26942== by 0x477532: dp_netdev_execute_actions (dpif-netdev.c:7349) > ==26942== by 0x477532: handle_packet_upcall (dpif-netdev.c:6630) > ==26942== by 0x477532: fast_path_processing (dpif-netdev.c:6726) > ==26942== by 0x47933C: dp_netdev_input__ (dpif-netdev.c:6814) > ==26942== by 0x479AB8: dp_netdev_input (dpif-netdev.c:6852) > ==26942== by 0x479AB8: dp_netdev_process_rxq_port (dpif-netdev.c:4287) > ==26942== by 0x47A6A9: dpif_netdev_run (dpif-netdev.c:5264) > ==26942== by 0x4324E7: type_run (ofproto-dpif.c:342) > ==26942== by 0x41C5FE: ofproto_type_run (ofproto.c:1734) > ==26942== by 0x40BAAC: bridge_run__ (bridge.c:2965) > ==26942== by 0x410CF3: bridge_run (bridge.c:3029) > ==26942== by 0x407614: main (ovs-vswitchd.c:127) > ==26942== Uninitialised value was created by a heap allocation > ==26942== at 0x4C2DB8F: malloc (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==26942== by 0x532574: xmalloc (util.c:138) > ==26942== by 0x46CD62: dp_packet_new (dp-packet.c:153) > ==26942== by 0x4A0431: eth_from_flow_str (netdev-dummy.c:1644) > ==26942== by 0x4A0431: netdev_dummy_receive (netdev-dummy.c:1783) > ==26942== by 0x531990: process_command (unixctl.c:308) > ==26942== by 0x531990: run_connection (unixctl.c:342) > ==26942== by 0x531990: unixctl_server_run (unixctl.c:393) > ==26942== by 0x40761E: main (ovs-vswitchd.c:128) > > 1316: ofproto-dpif - conntrack - tcp port reuse > > ==24039== Conditional jump or move depends on uninitialised value(s) > ==24039== at 0x587BF5: check_orig_tuple (conntrack.c:1004) > ==24039== by 0x587BF5: process_one (conntrack.c:1141) > ==24039== by 0x587BF5: conntrack_execute (conntrack.c:1220) > ==24039== by 0x47B02F: dp_execute_cb (dpif-netdev.c:7306) > ==24039== by 0x4AF7A6: odp_execute_actions (odp-execute.c:794) > ==24039== by 0x47755B: dp_netdev_execute_actions (dpif-netdev.c:7350) > ==24039== by 0x47755B: handle_packet_upcall (dpif-netdev.c:6631) > ==24039== by 0x47755B: fast_path_processing (dpif-netdev.c:6727) > ==24039== by 0x47935C: dp_netdev_input__ (dpif-netdev.c:6815) > ==24039== by 0x479AD8: dp_netdev_input (dpif-netdev.c:6853) > ==24039== by 0x479AD8: dp_netdev_process_rxq_port > (dpif-netdev.c:4287) > ==24039== by 0x47A6C9: dpif_netdev_run (dpif-netdev.c:5264) > ==24039== by 0x4324F7: type_run (ofproto-dpif.c:342) > ==24039== by 0x41C5FE: ofproto_type_run (ofproto.c:1734) > ==24039== by 0x40BAAC: bridge_run__ (bridge.c:2965) > ==24039== by 0x410CF3: bridge_run (bridge.c:3029) > ==24039== by 0x407614: main (ovs-vswitchd.c:127) > ==24039== Uninitialised value was created by a heap allocation > ==24039== at 0x4C2DB8F: malloc (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==24039== by 0x5325C4: xmalloc (util.c:138) > ==24039== by 0x46D144: dp_packet_new (dp-packet.c:153) > ==24039== by 0x46D144: dp_packet_new_with_headroom (dp-packet.c:163) > ==24039== by 0x51191E: eth_from_hex (packets.c:498) > ==24039== by 0x4A03B9: eth_from_packet (netdev-dummy.c:1609) > ==24039== by 0x4A03B9: netdev_dummy_receive (netdev-dummy.c:1765) > ==24039== by 0x5319E0: process_command (unixctl.c:308) > ==24039== by 0x5319E0: run_connection (unixctl.c:342) > ==24039== by 0x5319E0: unixctl_server_run (unixctl.c:393) > ==24039== by 0x40761E: main (ovs-vswitchd.c:128) > > According to comments in pkt_metadata_init(), conntrack data is valid > only if pkt_metadata.ct_state != 0. This patch prevents > check_orig_tuple() get called when conntrack data is uninitialized. > > Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com>
LGTM Acked-by: William Tu <u9012...@gmail.com> > --- > lib/conntrack.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index e5266e579452..86c16b2fbe77 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -1138,7 +1138,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, > handle_nat(pkt, conn, zone, ctx->reply, ctx->icmp_related); > } > > - } else if (check_orig_tuple(ct, pkt, ctx, now, &conn, nat_action_info)) { > + } else if (pkt->md.ct_state > + && check_orig_tuple(ct, pkt, ctx, now, &conn, > nat_action_info)) { > create_new_conn = conn_update_state(ct, pkt, ctx, conn, now); > } else { > if (ctx->icmp_related) { > -- > 2.7.4 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev