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: Avoid assertion failure for clone(ct(...bad actions...)).

Browse Source 
decode_NXAST_RAW_CT() temporarily pulls data off the beginning of its
ofpacts output ofpbuf and, on its error path, fails to push it back on.
At a higher layer, decode_NXAST_RAW_CLONE() asserts, via
ofpact_finish_CLONE(), that the ofpact_clone that it put is still in the
place where it put it, which causes an assertion failure.

The root cause here is the failure to re-push the clone header.  One could
fix that, but it would be pretty easy for that to go wrong again on some
other obscure error path.  Instead, this commit just makes the problem go
away by always saving and restoring 'ofpact->data' if a decode fails.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9862
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>
This commit is contained in:
Ben Pfaff
2018-08-15 14:57:13 -07:00
parent 60ebc04d12
commit 20cdd1dbd5
1 changed files with 10 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
lib/ofp-actions.c
View File

@@ -7500,6 +7500,7 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
uint64_t *ofpacts_tlv_bitmap)
{
const struct ofp_action_header *actions;
void *orig_data = ofpacts->data;
size_t orig_size = ofpacts->size;
enum ofperr error;
@@ -7519,14 +7520,17 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
error = ofpacts_decode(actions, actions_len, version, vl_mff_map,
ofpacts_tlv_bitmap, ofpacts);
if (error) {
ofpacts->size = orig_size;
return error;
if (!error) {
error = ofpacts_verify(ofpacts->data, ofpacts->size, allowed_ovsinsts,
outer_action);
}
error = ofpacts_verify(ofpacts->data, ofpacts->size, allowed_ovsinsts,
outer_action);
if (error) {
/* Back out changes to 'ofpacts'. (Normally, decoding would only
* append to 'ofpacts', so that one would expect only to need to
* restore 'ofpacts->size', but some action parsing temporarily pulls
* off data from the start of 'ofpacts' and may not properly re-push it
* on error paths.) */
ofpacts->data = orig_data;
ofpacts->size = orig_size;
}
return error;