Skip to content

Commit

Permalink
ofproto: Fix double-unref of temporary rule when learning.
Browse files Browse the repository at this point in the history
When ofproto_flow_mod_init() accepts a rule, it takes ownership of it and
either unrefs it on error or transfers ownership to the struct it
initializes on success, but ofproto_flow_mod_init_for_learn() was unref-ing
it a second time if it reported an error.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: William Tu <u9012063@gmail.com>
  • Loading branch information
blp committed Jan 26, 2018
1 parent c069da1 commit 8add6d8
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 10 deletions.
5 changes: 4 additions & 1 deletion ofproto/ofproto-provider.h
Expand Up @@ -1867,7 +1867,10 @@ struct rule_criteria {
/* flow_mod with execution context. */
struct ofproto_flow_mod {
/* Allocated by 'init' phase, may be freed after 'start' phase, as these
* are not needed for 'revert' nor 'finish'. */
* are not needed for 'revert' nor 'finish'.
*
* This structure owns a reference to 'temp_rule' (if it is nonnull) that
* must be eventually be released with ofproto_rule_unref(). */
struct rule *temp_rule;
struct rule_criteria criteria;
struct cls_conjunction *conjs;
Expand Down
13 changes: 4 additions & 9 deletions ofproto/ofproto.c
Expand Up @@ -4928,8 +4928,6 @@ ofproto_flow_mod_init_for_learn(struct ofproto *ofproto,
struct ofproto_flow_mod *ofm)
OVS_EXCLUDED(ofproto_mutex)
{
enum ofperr error;

/* Reject flow mods that do not look like they were generated by a learn
* action. */
if (fm->command != OFPFC_MODIFY_STRICT || fm->table_id == OFPTT_ALL
Expand Down Expand Up @@ -4970,13 +4968,7 @@ ofproto_flow_mod_init_for_learn(struct ofproto *ofproto,
}
}

/* Initialize ofproto_flow_mod for future use. */
error = ofproto_flow_mod_init(ofproto, ofm, fm, rule);
if (error) {
ofproto_rule_unref(rule);
return error;
}
return 0;
return ofproto_flow_mod_init(ofproto, ofm, fm, rule);
}

enum ofperr
Expand Down Expand Up @@ -7376,6 +7368,9 @@ ofproto_flow_mod_uninit(struct ofproto_flow_mod *ofm)
}
}

/* Initializes 'ofm' with 'ofproto', 'fm', and 'rule'. 'rule' may be null, but
* if it is nonnull then the caller must own a reference to it, which on
* success is transferred to 'ofm' and on failure is unreffed. */
static enum ofperr
ofproto_flow_mod_init(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
const struct ofputil_flow_mod *fm, struct rule *rule)
Expand Down

0 comments on commit 8add6d8

Please sign in to comment.