From 91644f45c6280b12f72af1f55bf8a83fccc04b1b Mon Sep 17 00:00:00 2001 From: William Tu Date: Wed, 6 Apr 2016 16:28:51 -0700 Subject: [PATCH] dp-packet: Fix use of uninitialised value at emc_lookup. Valgrind reports "Conditional jump or move depends on uninitialised value" and "Use of uninitialised value" at case 2016 ovn -- 3 HVs, 1 LS, 3 lports/HV. It is caused by 1) assigning an uninitialized value to 'key->hash' at emc_processing(). Due to uninit rss_hash_valid, dp_packet_rss_valid() might return true and undefined hash value is returned, and 2) at emc_lookup, the 'current_entry->key.hash' could be uninitialized due to dp_packet_clone(). The patch fixes the two and as a result, a couple of calls to dp_packet_rss_invalidate() become redundant and thus are removed. Call stacks: - Connditional jump or move depends on uninitialised value(s) dpif_netdev_packet_get_rss_hash (dpif-netdev.c:3334) emc_processing (dpif-netdev.c:3455) dp_netdev_input__ (dpif-netdev.c:3639) and, - Use of uninitialised value of size 8 emc_lookup (dpif-netdev.c:1785) emc_processing (dpif-netdev.c:3457) dp_netdev_input__ (dpif-netdev.c:3639) Signed-off-by: William Tu Signed-off-by: Daniele Di Proietto --- lib/dp-packet.c | 16 +++++++++++++++- lib/netdev-bsd.c | 1 - lib/netdev-dummy.c | 1 - lib/netdev-linux.c | 1 - 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/dp-packet.c b/lib/dp-packet.c index aec7fe7fb5a..0c85d508a23 100644 --- a/lib/dp-packet.c +++ b/lib/dp-packet.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum dp_packet_source so b->source = source; dp_packet_reset_offsets(b); pkt_metadata_init(&b->md, 0); + dp_packet_rss_invalidate(b); } static void @@ -167,6 +168,19 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom) new_buffer->l3_ofs = buffer->l3_ofs; new_buffer->l4_ofs = buffer->l4_ofs; new_buffer->md = buffer->md; +#ifdef DPDK_NETDEV + new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; +#else + new_buffer->rss_hash_valid = buffer->rss_hash_valid; +#endif + + if (dp_packet_rss_valid(new_buffer)) { +#ifdef DPDK_NETDEV + new_buffer->mbuf.hash.rss = buffer->mbuf.hash.rss; +#else + new_buffer->rss_hash = buffer->rss_hash; +#endif + } return new_buffer; } diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 75bd5a3c316..49c05f49938 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -641,7 +641,6 @@ netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet **packets, dp_packet_delete(packet); } else { dp_packet_pad(packet); - dp_packet_rss_invalidate(packet); packets[0] = packet; *c = 1; } diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index edc86fa1a47..a1013ff7a86 100644 --- a/lib/netdev-dummy.c +++ b/lib/netdev-dummy.c @@ -905,7 +905,6 @@ netdev_dummy_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet **arr, ovs_mutex_unlock(&netdev->mutex); dp_packet_pad(packet); - dp_packet_rss_invalidate(packet); arr[0] = packet; *c = 1; diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 3f5b6087914..a7d7ac7102d 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -1116,7 +1116,6 @@ netdev_linux_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet **packets, dp_packet_delete(buffer); } else { dp_packet_pad(buffer); - dp_packet_rss_invalidate(buffer); packets[0] = buffer; *c = 1; }