mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 14:25:26 +00:00
Code Issues Releases Wiki Activity

ofp-actions: Fix use-after-free in decode_NOTE.

Browse Source 
When decoding the 'note' action, variable-length data could be pushed to
a buffer immediately prior to calling ofpact_finish_NOTE(). The
ofpbuf_put() could cause reallocation, in which case the finish call
could access freed memory. Fix the issue by updating the local pointer
before passing it to ofpact_finish_NOTE().

If the memory was reused, it may trigger an assert in ofpact_finish():

assertion ofpact == ofpacts->header failed in ofpact_finish()

With the included test, make check-valgrind reports:

Invalid read of size 1
   at 0x500A9F: ofpact_finish_NOTE (ofp-actions.h:988)
   by 0x4FE5C1: decode_NXAST_RAW_NOTE (ofp-actions.c:4557)
   by 0x4FBC05: ofpact_decode (ofp-actions.inc2:3831)
   by 0x4F7E87: ofpacts_decode (ofp-actions.c:5780)
   by 0x4F709F: ofpacts_pull_openflow_actions__ (ofp-actions.c:5817)
   by 0x4F7856: ofpacts_pull_openflow_instructions (ofp-actions.c:6397)
   by 0x52CFF5: ofputil_decode_flow_mod (ofp-util.c:1727)
   by 0x5227A9: ofp_print_flow_mod (ofp-print.c:789)
   by 0x520823: ofp_to_string__ (ofp-print.c:3235)
   by 0x5204F6: ofp_to_string (ofp-print.c:3468)
   by 0x5925C8: do_recv (vconn.c:644)
   by 0x592372: vconn_recv (vconn.c:598)
   by 0x565CEA: rconn_recv (rconn.c:703)
   by 0x46CB62: ofconn_run (connmgr.c:1367)
   by 0x46C7AD: connmgr_run (connmgr.c:320)
   by 0x4224A9: ofproto_run (ofproto.c:1763)
   by 0x407C0D: bridge_run__ (bridge.c:2888)
   by 0x40767A: bridge_run (bridge.c:2943)
   by 0x4161B7: main (ovs-vswitchd.c:120)

Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ansis Atteka <ansisatteka@gmail.com>
This commit is contained in:
Joe Stringer
2016-04-28 14:13:38 -07:00
parent a78350d91a
commit ace39a6f63
2 changed files with 11 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
lib/ofp-actions.c
View File

@@ -4554,6 +4554,7 @@ decode_NXAST_RAW_NOTE(const struct nx_action_note *nan,
note = ofpact_put_NOTE(out);
note->length = length;
ofpbuf_put(out, nan->note, length);
note = out->header;
ofpact_finish_NOTE(out, &note);
return 0;

10
tests/ofproto-dpif.at
View File

@@ -782,6 +782,16 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
OVS_VSWITCHD_STOP
AT_CLEANUP
dnl As of OVS-2.5, a note action after 4 set_field actions are likely to
dnl trigger ofpbuf reallocation during decode (~1KB into ofpacts buffer).
dnl Using `make check-valgrind' here checks for use-after-free in this
dnl codepath.
AT_SETUP([ofproto-dpif - note action deep inside ofpacts])
OVS_VSWITCHD_START
AT_CHECK([ovs-ofctl add-flow br0 'actions=set_field:0x1->metadata,set_field:0x2->metadata,set_field:0x3->metadata,set_field:0x4->metadata,note:00000000000000000000000000000000,note:00000000000000000000000000000000'])
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([ofproto-dpif - output, OFPP_NONE ingress port])
OVS_VSWITCHD_START
add_of_ports br0 1 2