[ovs-git] Open vSwitch: ofproto-dpif-upcall: Avoid use-after-free in revalidate() corner cases. (master)

dev at openvswitch.org dev at openvswitch.org
Thu May 15 23:04:12 UTC 2014

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Open vSwitch".

The branch, master has been updated
       via  a6ce4b9d2518a261f0d8f51acef069d687b8df2e (commit)
       via  fbf4f74d8fb9184ebeceaf8338c834bcc839943a (commit)
       via  ecaf2f5c2f38d78a85ce6e8e895e288caff457a2 (commit)
       via  c17b52e8c3a97de1aaae8d940e0df4179912fd10 (commit)
      from  2c7ea58953128e7a8692a9f28d388d0311dda84d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit a6ce4b9d2518a261f0d8f51acef069d687b8df2e
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=a6ce4b9d2518a261f0d8f51acef069d687b8df2e
Author: Ben Pfaff <blp at nicira.com>
ofproto-dpif-upcall: Avoid use-after-free in revalidate() corner cases.
The loop in revalidate() needs to ensure that any data obtained from
dpif_flow_dump_next() is used before it is destroyed, as indicated by
dpif_flow_dump_next_may_destroy_keys().  In the common case, where
processing reaches the end of the main "while" loop, it does this, but
in two corner cases the code in the loop execute "continue;", which skipped
the check.  This commit fixes the problem.

Bug #1249988.
Signed-off-by: Ben Pfaff <blp at nicira.com>
Acked-by: Joe Stringer <joestringer at nicira.com>

commit fbf4f74d8fb9184ebeceaf8338c834bcc839943a
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=fbf4f74d8fb9184ebeceaf8338c834bcc839943a
Author: Simon Horman <horms at verge.net.au>
datapath: sample action without side effects
The sample action is rather generic, allowing arbitrary actions to be
executed based on a probability. However its use, within the Open vSwitch
code-base is limited: only a single user-space action is ever nested.

A consequence of the current implementation of sample actions is that
depending on weather the sample action executed (due to its probability)
any side-effects of nested actions may or may not be present before
executing subsequent actions.  This has the potential to complicate
verification of valid actions by the (kernel) datapath. And indeed adding
support for push and pop MPLS actions inside sample actions is one case
where such case.

In order to allow all supported actions to be continue to be nested inside
sample actions without the potential need for complex verification code
this patch changes the implementation of the sample action in the kernel
datapath so that sample actions are more like a function call and any side
effects of nested actions are not present when executing subsequent

With the above in mind the motivation for this change is twofold:

* To contain side-effects the sample action in the hope of making it
  easier to deal with in the future and;
* To avoid some rather complex verification code introduced in the MPLS
  datapath patch.

Some notes about the implementation:

* This patch silently changes the behaviour of sample actions whose nested
  actions have side-effects. There are no known users of such sample

* sample() does not clone the skb for the only known use-case of the sample
  action: a single nested userspace action. In such a case a clone is not
  needed as the userspace action has no side effects.

  Given that there are no known users of other nested actions and in order
  to avoid the complexity of predicting if other sequences of actions have
  side-effects in such cases the skb is cloned.

* As sample() provides a cloned skb in the unlikely case where there are
  nested actions other than a single userspace action it is no longer
  necessary to clone the skb in do_execute_actions() when executing a
  recirculation action just because the keep_skb parameter is set: this
  parameter was only set when processing the nested actions of a sample
  action.  Moreover it is possible to remove the keep_skb parameter of
  do_execute_actions entirely.

* As sample() provides either a cloned skb or one that has had a
  reference taken (using keep_skb) to do_execute_actions()
  the original skb passed to sample() is never consumed. Thus the
  caller of sample() (also do_execute_actions()) can use its generic
  error handling to free the skb on error.

Signed-off-by: Simon Horman <horms at verge.net.au>
Signed-off-by: Jesse Gross <jesse at nicira.com>

commit ecaf2f5c2f38d78a85ce6e8e895e288caff457a2
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=ecaf2f5c2f38d78a85ce6e8e895e288caff457a2
Author: Gurucharan Shetty <gshetty at nicira.com>
tests: Change to parse dynamically allocated ports on windows.
In Windows, we use kernel assigned TCP port for ssl/tcp and
unixctl. In tests, we parse the log files of ovsdb-server.log,
test-sflow.log and test-netflow.log to get this port. In all
the above cases, tcp port is allocated first and then the unixctl port.
So a 'head -1' on the result should be safe.

Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
Acked-by: Ben Pfaff <blp at nicira.com>

commit c17b52e8c3a97de1aaae8d940e0df4179912fd10
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=c17b52e8c3a97de1aaae8d940e0df4179912fd10
Author: Gurucharan Shetty <gshetty at nicira.com>
ovsdb-server.at: Skip transient transaction tests for Windows.
Windows port does not yet implement ovsdb-server's "--run" option.
Till it is implemented, skip the tests.

Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
Acked-by: Ben Pfaff <blp at nicira.com>


Summary of changes:
 datapath/actions.c            |   52 +++++++++++++++++++++++++++++------------
 ofproto/ofproto-dpif-upcall.c |    5 ++--
 tests/ofproto-dpif.at         |    8 +++----
 tests/ofproto-macros.at       |    4 ++--
 tests/ovsdb-idl.at            |    4 ++--
 tests/ovsdb-server.at         |   13 ++++++-----
 6 files changed, 55 insertions(+), 31 deletions(-)

Open vSwitch

More information about the git mailing list