Skip to content

Commit

Permalink
dp-packet: Fix use of uninitialised value at emc_lookup.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Daniele Di Proietto <[email protected]>
  • Loading branch information
williamtu authored and ddiproietto committed Apr 7, 2016
1 parent bc8f7c3 commit 91644f4
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 4 deletions.
16 changes: 15 additions & 1 deletion lib/dp-packet.c
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 0 additions & 1 deletion lib/netdev-bsd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 0 additions & 1 deletion lib/netdev-dummy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion lib/netdev-linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 91644f4

Please sign in to comment.