Skip to content

Commit

Permalink
net: openvswitch: limit the number of recursions from action sets
Browse files Browse the repository at this point in the history 
The ovs module allows for some actions to recursively contain an action
list for complex scenarios, such as sampling, checking lengths, etc.
When these actions are copied into the internal flow table, they are
evaluated to validate that such actions make sense, and these calls
happen recursively.

The ovs-vswitchd userspace won't emit more than 16 recursion levels
deep.  However, the module has no such limit and will happily accept
limits larger than 16 levels nested.  Prevent this by tracking the
number of recursions happening and manually limiting it to 16 levels
nested.

The initial implementation of the sample action would track this depth
and prevent more than 3 levels of recursion, but this was removed to
support the clone use case, rather than limited at the current userspace
limit.

Fixes: 798c166 ("openvswitch: Optimize sample action for the clone use cases")
Signed-off-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://lore.kernel.org/r/20240207132416.1488485-2-aconole@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Aaron Conole authored and Jakub Kicinski committed Feb 9, 2024
1 parent d02bfae commit 6e2f90d
Showing 1 changed file with 33 additions and 16 deletions.
49 changes: 33 additions & 16 deletions net/openvswitch/flow_netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ struct ovs_len_tbl {

#define OVS_ATTR_NESTED -1
#define OVS_ATTR_VARIABLE -2
#define OVS_COPY_ACTIONS_MAX_DEPTH 16

static bool actions_may_change_flow(const struct nlattr *actions)
{
Expand Down Expand Up @@ -2545,13 +2546,15 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
const struct sw_flow_key *key,
struct sw_flow_actions **sfa,
__be16 eth_type, __be16 vlan_tci,
u32 mpls_label_count, bool log);
u32 mpls_label_count, bool log,
u32 depth);

static int validate_and_copy_sample(struct net *net, const struct nlattr *attr,
const struct sw_flow_key *key,
struct sw_flow_actions **sfa,
__be16 eth_type, __be16 vlan_tci,
u32 mpls_label_count, bool log, bool last)
u32 mpls_label_count, bool log, bool last,
u32 depth)
{
const struct nlattr *attrs[OVS_SAMPLE_ATTR_MAX + 1];
const struct nlattr *probability, *actions;
Expand Down Expand Up @@ -2602,7 +2605,8 @@ static int validate_and_copy_sample(struct net *net, const struct nlattr *attr,
return err;

err = __ovs_nla_copy_actions(net, actions, key, sfa,
eth_type, vlan_tci, mpls_label_count, log);
eth_type, vlan_tci, mpls_label_count, log,
depth + 1);

if (err)
return err;
Expand All @@ -2617,7 +2621,8 @@ static int validate_and_copy_dec_ttl(struct net *net,
const struct sw_flow_key *key,
struct sw_flow_actions **sfa,
__be16 eth_type, __be16 vlan_tci,
u32 mpls_label_count, bool log)
u32 mpls_label_count, bool log,
u32 depth)
{
const struct nlattr *attrs[OVS_DEC_TTL_ATTR_MAX + 1];
int start, action_start, err, rem;
Expand Down Expand Up @@ -2660,7 +2665,8 @@ static int validate_and_copy_dec_ttl(struct net *net,
return action_start;

err = __ovs_nla_copy_actions(net, actions, key, sfa, eth_type,
vlan_tci, mpls_label_count, log);
vlan_tci, mpls_label_count, log,
depth + 1);
if (err)
return err;

Expand All @@ -2674,7 +2680,8 @@ static int validate_and_copy_clone(struct net *net,
const struct sw_flow_key *key,
struct sw_flow_actions **sfa,
__be16 eth_type, __be16 vlan_tci,
u32 mpls_label_count, bool log, bool last)
u32 mpls_label_count, bool log, bool last,
u32 depth)
{
int start, err;
u32 exec;
Expand All @@ -2694,7 +2701,8 @@ static int validate_and_copy_clone(struct net *net,
return err;

err = __ovs_nla_copy_actions(net, attr, key, sfa,
eth_type, vlan_tci, mpls_label_count, log);
eth_type, vlan_tci, mpls_label_count, log,
depth + 1);
if (err)
return err;

Expand Down Expand Up @@ -3063,7 +3071,7 @@ static int validate_and_copy_check_pkt_len(struct net *net,
struct sw_flow_actions **sfa,
__be16 eth_type, __be16 vlan_tci,
u32 mpls_label_count,
bool log, bool last)
bool log, bool last, u32 depth)
{
const struct nlattr *acts_if_greater, *acts_if_lesser_eq;
struct nlattr *a[OVS_CHECK_PKT_LEN_ATTR_MAX + 1];
Expand Down Expand Up @@ -3111,7 +3119,8 @@ static int validate_and_copy_check_pkt_len(struct net *net,
return nested_acts_start;

err = __ovs_nla_copy_actions(net, acts_if_lesser_eq, key, sfa,
eth_type, vlan_tci, mpls_label_count, log);
eth_type, vlan_tci, mpls_label_count, log,
depth + 1);

if (err)
return err;
Expand All @@ -3124,7 +3133,8 @@ static int validate_and_copy_check_pkt_len(struct net *net,
return nested_acts_start;

err = __ovs_nla_copy_actions(net, acts_if_greater, key, sfa,
eth_type, vlan_tci, mpls_label_count, log);
eth_type, vlan_tci, mpls_label_count, log,
depth + 1);

if (err)
return err;
Expand Down Expand Up @@ -3152,12 +3162,16 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
const struct sw_flow_key *key,
struct sw_flow_actions **sfa,
__be16 eth_type, __be16 vlan_tci,
u32 mpls_label_count, bool log)
u32 mpls_label_count, bool log,
u32 depth)
{
u8 mac_proto = ovs_key_mac_proto(key);
const struct nlattr *a;
int rem, err;

if (depth > OVS_COPY_ACTIONS_MAX_DEPTH)
return -EOVERFLOW;

nla_for_each_nested(a, attr, rem) {
/* Expected argument lengths, (u32)-1 for variable length. */
static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
Expand Down Expand Up @@ -3355,7 +3369,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
err = validate_and_copy_sample(net, a, key, sfa,
eth_type, vlan_tci,
mpls_label_count,
log, last);
log, last, depth);
if (err)
return err;
skip_copy = true;
Expand Down Expand Up @@ -3426,7 +3440,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
err = validate_and_copy_clone(net, a, key, sfa,
eth_type, vlan_tci,
mpls_label_count,
log, last);
log, last, depth);
if (err)
return err;
skip_copy = true;
Expand All @@ -3440,7 +3454,8 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
eth_type,
vlan_tci,
mpls_label_count,
log, last);
log, last,
depth);
if (err)
return err;
skip_copy = true;
Expand All @@ -3450,7 +3465,8 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
case OVS_ACTION_ATTR_DEC_TTL:
err = validate_and_copy_dec_ttl(net, a, key, sfa,
eth_type, vlan_tci,
mpls_label_count, log);
mpls_label_count, log,
depth);
if (err)
return err;
skip_copy = true;
Expand Down Expand Up @@ -3495,7 +3511,8 @@ int ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,

(*sfa)->orig_len = nla_len(attr);
err = __ovs_nla_copy_actions(net, attr, key, sfa, key->eth.type,
key->eth.vlan.tci, mpls_label_count, log);
key->eth.vlan.tci, mpls_label_count, log,
0);
if (err)
ovs_nla_free_flow_actions(*sfa);

Expand Down

0 comments on commit 6e2f90d

Please sign in to comment.