> On Feb 3, 2014, at 10:38 AM, Jesse Gross <[email protected]> wrote: > >> On Fri, Jan 24, 2014 at 2:58 PM, Jarno Rajahalme <[email protected]> >> wrote: >> diff --git a/datapath/datapath.c b/datapath/datapath.c >> index 92ae66a..5b12a5d 100644 >> --- a/datapath/datapath.c >> +++ b/datapath/datapath.c >> @@ -817,11 +840,27 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff >> *skb, struct genl_info *info) >> OVS_NLERR("Flow actions may not be safe on all >> matching packets.\n"); >> goto err_kfree; >> } >> - } else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) { >> + } else if (cmd == OVS_FLOW_CMD_NEW) { >> error = -EINVAL; >> goto error; >> } >> + /* XXX: Is it OK to SET actions to NULL? */ > > Yes, we already do this so it's OK as long as the length is also set to 0.
I don't immediately see how this is safe, as we seem to happily dereference the sf_acts pointer without ever checking if it is NULL, like in "acts->actions_len". It seems to me that we should allocate a new sf_flow_actions in any case for SET, and make the actions_len zero if no actions are passed in by the userspace. Jarno _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
