[ovs-git] [openvswitch/ovs] 8b8ef5: tests: Add bundle action test with buffer realloc.

GitHub noreply at github.com
Thu Mar 3 21:55:14 UTC 2016


  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: 8b8ef592521e32d0e32581bf39c5d2a5cd445977
      https://github.com/openvswitch/ovs/commit/8b8ef592521e32d0e32581bf39c5d2a5cd445977
  Author: Joe Stringer <joe at ovn.org>
  Date:   2016-03-03 (Thu, 03 Mar 2016)

  Changed paths:
    M tests/bundle.at

  Log Message:
  -----------
  tests: Add bundle action test with buffer realloc.

Add a test which causes internal reallocation of the ofpacts buffer,
followed by a large bundle action which should cause a subsequent
reallocation while decoding slave ports. Running this test under
valgrind reveals the issue below, which is fixed in the following
commit.

Invalid read of size 4
   at 0x4CED87: decode_bundle (ofp-actions.c:1253)
   by 0x4CEDFC: decode_NXAST_RAW_BUNDLE (ofp-actions.c:1272)
   by 0x4DBDE6: ofpact_decode (ofp-actions.inc2:3765)
   by 0x4D6914: ofpacts_decode (ofp-actions.c:5735)
   by 0x4D6A3D: ofpacts_pull_openflow_actions__ (ofp-actions.c:5772)
   by 0x4D74F3: ofpacts_pull_openflow_instructions (ofp-actions.c:6352)
   by 0x4F59FA: ofputil_decode_flow_mod (ofp-util.c:1704)
   by 0x4EAD18: ofp_print_flow_mod (ofp-print.c:786)
   by 0x4F0711: ofp_to_string__ (ofp-print.c:3220)
   by 0x4F0D98: ofp_to_string (ofp-print.c:3453)
   by 0x5486B3: do_recv (vconn.c:644)
   by 0x548498: vconn_recv (vconn.c:598)
   by 0x524582: rconn_recv (rconn.c:703)
   by 0x45DA61: ofconn_run (connmgr.c:1370)
   by 0x45B3B4: connmgr_run (connmgr.c:323)
   by 0x41D1E8: ofproto_run (ofproto.c:1762)
   by 0x40CEE0: bridge_run__ (bridge.c:2885)
   by 0x40D093: bridge_run (bridge.c:2940)
   by 0x412F7E: main (ovs-vswitchd.c:120)
Address 0x66aa460 is 1,152 bytes inside a block of size 1,184 free'd
   at 0x4C2AF2E: realloc (vg_replace_malloc.c:692)
   by 0x543D27: xrealloc (util.c:123)
   by 0x5089EF: ofpbuf_resize__ (ofpbuf.c:243)
   by 0x508B81: ofpbuf_prealloc_tailroom (ofpbuf.c:290)
   by 0x508D5C: ofpbuf_put_uninit (ofpbuf.c:364)
   by 0x508DEF: ofpbuf_put (ofpbuf.c:387)
   by 0x4CED7D: decode_bundle (ofp-actions.c:1255)
   by 0x4CEDFC: decode_NXAST_RAW_BUNDLE (ofp-actions.c:1272)
   by 0x4DBDE6: ofpact_decode (ofp-actions.inc2:3765)
   by 0x4D6914: ofpacts_decode (ofp-actions.c:5735)
   by 0x4D6A3D: ofpacts_pull_openflow_actions__ (ofp-actions.c:5772)
   by 0x4D74F3: ofpacts_pull_openflow_instructions (ofp-actions.c:6352)
   by 0x4F59FA: ofputil_decode_flow_mod (ofp-util.c:1704)
   by 0x4EAD18: ofp_print_flow_mod (ofp-print.c:786)
   by 0x4F0711: ofp_to_string__ (ofp-print.c:3220)
   by 0x4F0D98: ofp_to_string (ofp-print.c:3453)
   by 0x5486B3: do_recv (vconn.c:644)
   by 0x548498: vconn_recv (vconn.c:598)
   by 0x524582: rconn_recv (rconn.c:703)
   by 0x45DA61: ofconn_run (connmgr.c:1370)

Signed-off-by: Joe Stringer <joe at ovn.org>
Acked-by: Jarno Rajahalme <jarno at ovn.org>


  Commit: 19b58f3cbcb3191432eefba3a504376399cc07a7
      https://github.com/openvswitch/ovs/commit/19b58f3cbcb3191432eefba3a504376399cc07a7
  Author: Joe Stringer <joe at ovn.org>
  Date:   2016-03-03 (Thu, 03 Mar 2016)

  Changed paths:
    M lib/ofp-actions.c

  Log Message:
  -----------
  ofp-actions: Fix use-after-free in bundle action.

If the actions list in an incoming flow mod is long enough, and there is
a bundle() action with 3 or more slaves, then it is possible for a
reallocation to occur after placing the ofpact_bundle into the ofpacts
buffer, while slave ports into the buffer. If the memory freed by this
reallocation is then passed to another thread, then that thread may
modify the value that bundle->n_slaves points to. If this occurs quickly
enough before the main thread finishes copying all of the slaves, then
the iteration may continue beyond the originally intended number of
slaves, copying (and swapping) an undetermined number of 2-byte chunks
from the openflow message. Finally, the length of the ofpact will be
updated based on how much data was written to the buffer, which may be
significantly longer than intended.

In many cases, the freed memory may not be allocated to another thread
and be left untouched. In some milder bug cases, this will lead to
'bundle' actions using more memory than required. In more serious cases,
this length may then exceed the maximum length of an OpenFlow action,
which is then stored (truncated) into the 16-bit length field in the
ofpact header. Later execution of ofpacts_verify() would then use this
length to iterate through the ofpacts, and may dereference memory in
unintended ways, causing crashes or infinite loops by attempting to
parse/validate arbitrary data as ofpact objects.

Fix the issue by updating 'bundle' within the iteration, immediately
after (potentially) expanding the bundle.

Thanks to Jarno Rajahalme for his keen pair of eyes on finding this
issue.

VMWare-BZ: #1614715
Fixes: f25d0cf3c366 ("Introduce ofpacts, an abstraction of OpenFlow actions.")
Signed-off-by: Joe Stringer <joe at ovn.org>
Acked-by: Jarno Rajahalme <jarno at ovn.org>


  Commit: 5308056f53406e75d211f73a2847f9ebdf9c91c8
      https://github.com/openvswitch/ovs/commit/5308056f53406e75d211f73a2847f9ebdf9c91c8
  Author: Joe Stringer <joe at ovn.org>
  Date:   2016-03-03 (Thu, 03 Mar 2016)

  Changed paths:
    M lib/ofp-actions.c

  Log Message:
  -----------
  ofp-actions: Prevent integer overflow in decode.

When decoding a variable-length action, if the length of the action
exceeds the length storable in a uint16_t then something has gone
terribly wrong. Assert that this is not the case.

Signed-off-by: Joe Stringer <joe at ovn.org>
Acked-by: Jarno Rajahalme <jarno at ovn.org>


  Commit: c391558c30d7d7eb5c20da3ba0cd4700b6890962
      https://github.com/openvswitch/ovs/commit/c391558c30d7d7eb5c20da3ba0cd4700b6890962
  Author: Joe Stringer <joe at ovn.org>
  Date:   2016-03-03 (Thu, 03 Mar 2016)

  Changed paths:
    M lib/ofpbuf.c

  Log Message:
  -----------
  ofpbuf: Use ptrdiff_t for pointer delta.

Signed-off-by: Joe Stringer <joe at ovn.org>
Acked-by: Jarno Rajahalme <jarno at ovn.org>


Compare: https://github.com/openvswitch/ovs/compare/3a103e4a99d1...c391558c30d7


More information about the git mailing list