ofp-actions: Fix use-after-free in bundle action.
authorJoe Stringer <joe@ovn.org>
Thu, 3 Mar 2016 08:22:49 +0000 (21:22 +1300)
committerJoe Stringer <joe@ovn.org>
Thu, 3 Mar 2016 21:55:07 +0000 (13:55 -0800)
commit19b58f3cbcb3191432eefba3a504376399cc07a7
treeba368c3a965f85209fb20707d4045b86e4fa31b9
parent8b8ef592521e32d0e32581bf39c5d2a5cd445977
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@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>
lib/ofp-actions.c