[ovs-dev] [patch v1] conntrack: Fix 'check_orig_tuple()' Valgrind false positive.

[Darrell Ball]

Valgrind reported that 'pkt->md.ct_orig_tuple.ipv4.ipv4_proto' is
uninitialized in 'check_orig_tuple()', if 'ct_state' is zero.  Although
this is true, the check is superceded, as even if it succeeds the check
for natted packets based on 'ct_state' is an ORed condition and is intended
to catch this case.
The check is '!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT))' which
filters out all packets excepted natted ones.  Move this check up to
prevent the Valgrind complaint, which also helps performance and also remove
recenlty added redundant check adding extra cycles.

Fixes: f44733c527da ("conntrack: Validate accessing of conntrack data in pkt_metadata.")
CC: Yifeng Sun <pkusunyifeng at gmail.com>
Signed-off-by: Darrell Ball <dlu998 at gmail.com>
---
 lib/conntrack.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index fd71e6c..b56ef06 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1005,11 +1005,11 @@ check_orig_tuple(struct conntrack *ct, struct dp_packet *pkt,
                  struct conn **conn,
                  const struct nat_action_info_t *nat_action_info)
 {
-    if ((ctx_in->key.dl_type == htons(ETH_TYPE_IP) &&
+    if (!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
+        (ctx_in->key.dl_type == htons(ETH_TYPE_IP) &&
          !pkt->md.ct_orig_tuple.ipv4.ipv4_proto) ||
         (ctx_in->key.dl_type == htons(ETH_TYPE_IPV6) &&
          !pkt->md.ct_orig_tuple.ipv6.ipv6_proto) ||
-        !(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
         nat_action_info) {
         return false;
     }
@@ -1142,8 +1142,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
             handle_nat(pkt, conn, zone, ctx->reply, ctx->icmp_related);
         }
 
-    } else if (pkt->md.ct_state
-               && check_orig_tuple(ct, pkt, ctx, now, &conn, nat_action_info)) {
+    } else if (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) {
-- 
1.9.1