1. 22 Mar, 2016 2 commits
    • Justin Pettit's avatar
      Set release date for 2.4.1. · f8184f0b
      Justin Pettit authored
      Signed-off-by: default avatarJustin Pettit <jpettit@ovn.org>
      Acked-by: default avatarBen Pfaff <blp@ovn.org>
      f8184f0b
    • Ben Pfaff's avatar
      flow: Fix remote DoS for crafted MPLS packets with debug logging enabled. · d74cf9d7
      Ben Pfaff authored
      A crafted MPLS packet yields a zero 'count' in this excerpt from
      miniflow_extract():
      
              count = parse_mpls(&data, &size);
              miniflow_push_words_32(mf, mpls_lse, mpls, count);
      
      In turn, miniflow_push_words_32() updated mf.map as follows:
      
          MF.map |= ((UINT64_MAX >> (64 - DIV_ROUND_UP(N_WORDS, 2))) << ofs64);
      
      which expanded to:
      
          mf.map |= (UINT64_MAX >> 64) << ofs64;
      
      Unforunately, C renders shifting a 64-bit constant by 64 bits undefined.
      On common x86 platforms, 'n << 64' is equal to 'n', so this behaves as:
      
          mf.map |= UINT64_MAX << ofs64;
      
      In this particular case, ofs64 is 15, so this sets the most-significant 48
      bits of mf.map (a 63-bit bit-field) to 1.  Only the least-significant 28
      bits of mf.map should ever be set to 1, so this sets 35 bits to 1 that
      should never be.  Because of the structure of the data structure that
      mf.map is embedded within, this makes it possible later to overwrite 8*35
      == 280 bytes of data in the stack.  However, there is no obvious way to
      control the data used in the overwrite--it is memcpy'd from one place to
      another but the source data does not come from the network.  In the bug
      reporter's testing, this overwrite caused a userspace crash if debug
      logging was enabled, but not otherwise.
      
      This commit fixes the problem by avoiding the out-of-range shift.
      
      Vulnerability: CVE-2016-2074
      Reported-by: default avatarKashyap Thimmaraju <kashyap.thimmaraju@sec.t-labs.tu-berlin.de>
      Reported-by: default avatarBhargava Shastry <bshastry@sec.t-labs.tu-berlin.de>
      Signed-off-by: default avatarBen Pfaff <blp@ovn.org>
      Acked-by: default avatarJesse Gross <jesse@kernel.org>
      d74cf9d7
  2. 25 Mar, 2016 1 commit
    • Daniele Di Proietto's avatar
      ovs-thread: Do not always end quiescent state in ovs_thread_create(). · 4e23657c
      Daniele Di Proietto authored
      A new thread must be started in a non quiescent state.  There is a call
      to ovsrcu_quiesce_end() in ovsthread_wrapper(), to enforce this.
      
      ovs_thread_create(), instead, is executed in the parent thread. It must
      call ovsrcu_quiesce_end() on its first invocation, to put the main
      thread in a non quiescent state.  On every other invocation, it doesn't
      make sense to alter the calling thread state, so this commits wraps the
      call to ovsrcu_quiesce_end() in an ovsthread_once construct.
      
      This fixes a bug in ovs-rcu where the first call in the process to
      ovsrcu_quiesce_start() will not be honored, because the calling thread
      will need to create the 'urcu' thread (and creating a thread will
      wrongly end its quiescent state).
      
      ovsrcu_quiesce_start()
        ovs_rcu_quiesced()
          if (ovsthread_once_start(&once)) {
              ovs_thread_create("urcu") /*This will end the quiescent state*/
          }
      
      This bug affects in particular ovs-vswitchd with DPDK.
      In the DPDK case the first threads created are "vhost_thread" and
      "dpdk_watchdog".  If dpdk_watchdog is the first to call
      ovsrcu_quiesce_start() (via xsleep()), the call is not honored and
      the RCU grace period lasts at least for DPDK_PORT_WATCHDOG_INTERVAL
      (5s on current master).  If vhost_thread, on the other hand, is the
      first to call ovsrcu_quiesce_start(), the call is not honored and the
      RCU grace period lasts undefinitely, because no more calls to
      ovsrcu_quiesce_start() are issued from vhost_thread.
      
      For some reason (it's a race condition after all), on current master,
      dpdk_watchdog will always be the first to call ovsrcu_quiesce_start(),
      but with the upcoming DPDK database configuration changes, sometimes
      vhost_thread will issue the first call to ovsrcu_quiesce_start().
      
      Sample ovs-vswitchd.log:
      
      2016-03-23T22:34:28.532Z|00004|ovs_rcu(urcu3)|WARN|blocked 8000 ms
      waiting for vhost_thread2 to quiesce
      2016-03-23T22:34:30.501Z|00118|ovs_rcu|WARN|blocked 8000 ms waiting for
      vhost_thread2 to quiesce
      2016-03-23T22:34:36.532Z|00005|ovs_rcu(urcu3)|WARN|blocked 16000 ms
      waiting for vhost_thread2 to quiesce
      2016-03-23T22:34:38.501Z|00119|ovs_rcu|WARN|blocked 16000 ms waiting for
      vhost_thread2 to quiesce
      
      The commit also adds a test for the ovs-rcu module to make sure that:
      * A new thread is started in a non quiescent state.
      * The first call to ovsrcu_quiesce_start() is honored.
      * When a process becomes multithreaded the main thread is put in an
        active state
      Signed-off-by: default avatarDaniele Di Proietto <diproiettod@vmware.com>
      Acked-by: default avatarBen Pfaff <blp@ovn.org>
      4e23657c
  3. 23 Mar, 2016 2 commits
  4. 22 Mar, 2016 1 commit
    • Ian Stokes's avatar
      bridge: Fix qos_unixctl_show bug. · cb8cdcd7
      Ian Stokes authored
      netdev_get_qos returns a value to indicate if an error has occurred while
      attempting to query the QoS configuration of an interface. If an error does
      occur the pointer argument passed to it will be set to null before returning.
      Currently the vswitch will segfault if this occurs as qos_unixctl_show will
      attempt to access the pointer directly after it calls netdev_get_qos.
      
      Avoid this by adding a check for the return value and flagging an appropriate
      error message to appctl.
      Signed-off-by: default avatarIan Stokes <ian.stokes@intel.com>
      [blp@ovn.org changed details of error report]
      Signed-off-by: default avatarBen Pfaff <blp@ovn.org>
      cb8cdcd7
  5. 17 Mar, 2016 1 commit
  6. 11 Mar, 2016 3 commits
  7. 08 Mar, 2016 1 commit
  8. 07 Mar, 2016 1 commit
    • William Tu's avatar
      ofp-util: Fix use-after-free in group append. · 23fba242
      William Tu authored
      Upstream commit ef5774e3.
      
      It is possible for ofpbuf_put() to realloc a newly allocated address,
      casuing the previously referenced pointer, ogds, points to old/free'd
      address. The issue is generated by forcing ofpbuf_put() to use newly
      allocated buffer and valgrind reports invalid write. The similiar syndrome
      is reported at: https://patchwork.ozlabs.org/patch/591330/
      
      Invalid write of size 2
          ofputil_append_ofp15_group_desc_reply (ofp-util.c:8367)
          ofputil_append_group_desc_reply (ofp-util.c:8392)
          append_group_desc (ofproto.c:6262)
          handle_group_request (ofproto.c:6230)
          handle_group_desc_stats_request (ofproto.c:6269)
          handle_openflow__ (ofproto.c:7337)
          handle_openflow (ofproto.c:7403)
          ofconn_run (connmgr.c:1379)
          connmgr_run (connmgr.c:323)
          ofproto_run (ofproto.c:1762)
          bridge_run__ (bridge.c:2885)
          bridge_run (bridge.c:2940)
          main (ovs-vswitchd.c:120)
      
      Address 0x7cb1020 is 144 bytes inside a block of size 1,144 free'd
          free (vg_replace_malloc.c:530)
          ofpbuf_resize__ (ofpbuf.c:246)
          ofpbuf_put (ofpbuf.c:386)
          nx_put_header__ (nx-match.c:1241)
          nxm_put__ (nx-match.c:697)
          oxm_put_field_array (nx-match.c:1226)
          ofputil_put_group_prop_ntr_selection_method (ofp-util.c:8305)
          ofputil_append_ofp15_group_desc_reply (ofp-util.c:8364)
          ofputil_append_group_desc_reply (ofp-util.c:8392)
          append_group_desc (ofproto.c:6262)
      Signed-off-by: default avatarWilliam Tu <u9012063@gmail.com>
      Signed-off-by: default avatarJoe Stringer <joe@ovn.org>
      23fba242
  9. 03 Mar, 2016 2 commits
    • Joe Stringer's avatar
      ofp-actions: Prevent integer overflow in decode. · 1fce5274
      Joe Stringer authored
      Upstream commit 5308056f.
      
      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: default avatarJoe Stringer <joe@ovn.org>
      Acked-by: default avatarJarno Rajahalme <jarno@ovn.org>
      1fce5274
    • Joe Stringer's avatar
      ofp-actions: Fix use-after-free in bundle action. · 0bdf8e23
      Joe Stringer authored
      Upstream commit 19b58f3c.
      
      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: f25d0cf3 ("Introduce ofpacts, an abstraction of OpenFlow actions.")
      Signed-off-by: default avatarJoe Stringer <joe@ovn.org>
      Acked-by: default avatarJarno Rajahalme <jarno@ovn.org>
      0bdf8e23
  10. 16 Feb, 2016 2 commits
  11. 06 Feb, 2016 2 commits
  12. 05 Feb, 2016 2 commits
  13. 04 Feb, 2016 1 commit
  14. 03 Feb, 2016 1 commit
    • Ben Pfaff's avatar
      ofproto: Detect and handle errors in ofproto_port_add(). · 4034cac0
      Ben Pfaff authored
      The update_port() function called in ofproto_port_add() can encounter
      errors that prevent a port from being added, but nothing was checking for
      the error and in fact update_port() didn't even pass the error along to
      its caller.  This commit fixes the problem.
      
      The scenario that led me to examine this code can be triggered as follows
      from the sandbox, as long as you change --enable-dummy=override to
      --enable-dummy=system in ovs-sandbox:
      
      ovs-vsctl add-br br0
      ovs-vsctl add-port br0 tun0 \
          -- set interface tun0 type=stt options:remote_ip=1.2.3.4
      ovs-vsctl add-port br0 tun1 \
          -- set interface tun1 type=stt options:remote_ip=1.2.3.4
      
      The second add-port will fail due to the duplicate tunnel options, but
      ofproto_port_add() will not return the error.  Instead, it will report to
      the caller that it succeeded and tell it that it has ofp_port OFPP_NONE
      (65535), which is invalid and it obviously does not.  The result is that
      you get bizarre log messages like this:
      
          tunnel|WARN|tun1: attempting to add tunnel port with same config as port 'tun0' (::->1.2.3.4, key=0, dp port=7471, pkt mark=0)
          ofproto|WARN|br0: could not add port tun1 (File exists)
          bridge|INFO|bridge br0: added interface tun1 on port 65535
          ofproto|WARN|br0: cannot configure bfd on nonexistent port 65535
          ofproto|WARN|br0: cannot configure LLDP on nonexistent port 65535
          ofproto|WARN|br0: cannot get STP status on nonexistent port 65535
          ofproto|WARN|br0: cannot get RSTP status on nonexistent port 65535
          ofproto|WARN|br0: cannot get STP stats on nonexistent port 65535
          ofproto|WARN|br0: cannot get STP stats on nonexistent port 65535
      
      VMware-BZ: #1598643
      Signed-off-by: default avatarBen Pfaff <blp@ovn.org>
      Acked-by: default avatarJustin Pettit <jpettit@ovn.org>
      4034cac0
  15. 01 Feb, 2016 1 commit
  16. 29 Jan, 2016 1 commit
  17. 27 Jan, 2016 1 commit
  18. 25 Jan, 2016 1 commit
    • Ben Pfaff's avatar
      ofproto-dpif-xlate: Fix recirculation for resubmit to current table. · 9cd7938f
      Ben Pfaff authored
      When recirculation defers actions for processing later, it decides
      based on the actions being saved whether it needs to record the table
      and cookie from which they originated.  Until now, it was thought that
      this was only important for actions that send packets to the controller
      (because those actions send the table ID and cookie).  This overlooked
      a special case of the "resubmit" action which also depends on the
      current table ID, which meant that this special case malfunctioned if
      it came after recirculation.  This commit fixes the problem.
      
      This is a backport of a fix orginally committed on master.  That fix
      was able to add a test, but branch-2.4 lacks the "debug_recirc" feature
      needed for the test.
      
      Found while testing another feature under development.
      Signed-off-by: default avatarBen Pfaff <blp@ovn.org>
      Acked-by: default avatarJarno Rajahalme <jarno@ovn.org>
      9cd7938f
  19. 20 Jan, 2016 1 commit
    • Ben Pfaff's avatar
      ofproto: Fix memory leak and memory exhaustion bugs in group_mod. · a7a43b43
      Ben Pfaff authored
      In handle_group_mod() cases where adding a group failed, nothing freed the
      list of buckets, causing a leak.  The same was true in every case of
      modifying a group.  This commit fixes the problem by changing add_group()
      to never steal or free the buckets (modify_group() already acted this way)
      and then making handle_group_mod() always free the buckets when it's done.
      
      This approach might at first raise objections, because it makes add_group()
      copy the buckets instead of just take the existing ones.  On branch-2.5
      and master, there's a good reason for that--please see the original commit
      for explanation.  On this backport to branch-2.4, though, we just use this
      approach to avoid having to carefully write a new version for the backport.
      
      Found by pain and suffering.
      Signed-off-by: default avatarBen Pfaff <blp@ovn.org>
      Acked-by: default avatarJarno Rajahalme <jarno@ovn.org>
      a7a43b43
  20. 11 Jan, 2016 2 commits
  21. 06 Jan, 2016 1 commit
  22. 05 Jan, 2016 1 commit
  23. 04 Jan, 2016 3 commits
  24. 23 Dec, 2015 2 commits
  25. 22 Dec, 2015 1 commit
  26. 21 Dec, 2015 2 commits
  27. 11 Dec, 2015 1 commit