When handle_flow_miss() saw that subfacet did not have any actions, then
the associated packet would get freed early, in the loop that constructs
the set of batched operations. However, there would still be a "flow_put"
operation that referenced the key that shares the same memory block as the
packet. The memory allocator would overwrite the first few bytes of this
block, causing bizarre errors in the flow_put.
This commit changes the memory release strategy to be less error-prone, by
deferring all freeing of packets to the end of the function. With this
change, every packet gets freed in the same place, instead of having some
packets freed in one place and other packets freed in another.
Here is the valgrind report that pinpoints the problem:
Invalid read of size 4
at 0x4026838: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
by 0x80E9B52: dpif_linux_flow_to_ofpbuf (dpif-linux.c:1714)
by 0x80E9C77: dpif_linux_operate (dpif-linux.c:883)
by 0x80AFB5A: dpif_operate (dpif.c:994)
by 0x809A03B: handle_upcalls (ofproto-dpif.c:2758)
by 0x809A23A: run_fast (ofproto-dpif.c:757)
by 0x808C04E: ofproto_run_fast (ofproto.c:963)
by 0x806DFB6: bridge_run_fast (bridge.c:1811)
by 0x8074B59: main (ovs-vswitchd.c:98)
Address 0x4427948 is 80 bytes inside a block of size 2,048 free'd
at 0x402421C: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
by 0x80CD865: ofpbuf_delete (ofpbuf.c:187)
by 0x80CD8AA: ofpbuf_list_delete (ofpbuf.c:531)
by 0x8099F06: handle_upcalls (ofproto-dpif.c:2747)
by 0x809A23A: run_fast (ofproto-dpif.c:757)
by 0x808C04E: ofproto_run_fast (ofproto.c:963)
by 0x806DFB6: bridge_run_fast (bridge.c:1811)
by 0x8074B59: main (ovs-vswitchd.c:98)
Bug #9346.
Reported-by: Alan Shieh <ashieh@nicira.com>
Reported-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
- list_remove(&packet->list_node);
if (flow->vlan_tci != subfacet->initial_tci) {
/* This packet was received on a VLAN splinter port. We added
* a VLAN to the packet to make the packet resemble the flow,
if (flow->vlan_tci != subfacet->initial_tci) {
/* This packet was received on a VLAN splinter port. We added
* a VLAN to the packet to make the packet resemble the flow,
/* Process each element in the to-do list, constructing the set of
* operations to batch. */
n_ops = 0;
/* Process each element in the to-do list, constructing the set of
* operations to batch. */
n_ops = 0;
- HMAP_FOR_EACH_SAFE (miss, next_miss, hmap_node, &todo) {
+ HMAP_FOR_EACH (miss, hmap_node, &todo) {
handle_flow_miss(ofproto, miss, flow_miss_ops, &n_ops);
handle_flow_miss(ofproto, miss, flow_miss_ops, &n_ops);
- ofpbuf_list_delete(&miss->packets);
- hmap_remove(&todo, &miss->hmap_node);
- free(miss);
}
assert(n_ops <= ARRAY_SIZE(flow_miss_ops));
}
assert(n_ops <= ARRAY_SIZE(flow_miss_ops));
/* Execute batch. */
for (i = 0; i < n_ops; i++) {
/* Execute batch. */
for (i = 0; i < n_ops; i++) {
if (op->subfacet->actions != execute->actions) {
free((struct nlattr *) execute->actions);
}
if (op->subfacet->actions != execute->actions) {
free((struct nlattr *) execute->actions);
}
- ofpbuf_delete((struct ofpbuf *) execute->packet);
break;
case DPIF_OP_FLOW_PUT:
break;
case DPIF_OP_FLOW_PUT:
+ HMAP_FOR_EACH_SAFE (miss, next_miss, hmap_node, &todo) {
+ ofpbuf_list_delete(&miss->packets);
+ hmap_remove(&todo, &miss->hmap_node);
+ free(miss);
+ }
+ hmap_destroy(&todo);