net: openvswitch: reject oversized nested action attrs

Open vSwitch stores generated flow actions as nlattrs, whose nla_len
field is u16. Commit a1e64addf3ff ("net: openvswitch: remove
misbehaving actions length check") allowed the total sw_flow_actions
stream to grow beyond 64 KiB, which is valid, but also removed the last
guard preventing a generated nested action attribute from exceeding
U16_MAX.

An oversized generated container can thus be closed with a truncated
nla_len. A later dump or teardown then walks a structurally different
stream than the one that was validated. In particular, an oversized
nested CLONE/CT action may cause subsequent bytes in the generated
stream to be interpreted as independent actions.

Keep the larger total-action-stream behavior, but make nested action
close reject generated containers that do not fit in nla_len, and return
the error through all callers. For recursive SAMPLE, CLONE, DEC_TTL, and
CHECK_PKT_LEN builders, trim resource-owning action-list tails in reverse
construction order before discarding failed wrappers, so resources copied
into the rejected tails are released before the wrappers are removed.

Most failed outer wrappers are discarded by truncating actions_len after
child resources have been released. CHECK_PKT_LEN also trims its parent
after branch resources are gone. SET/TUNNEL close failures unwind their
known tun_dst ownership directly, and SET_TO_MASKED has no external
ownership and truncates on close failure.

Fixes: a1e64addf3ff ("net: openvswitch: remove misbehaving actions length check")
Cc: stable@vger.kernel.org
Assisted-by: avom-custom-harness:gpt-5.5-qwen3.6-mod-mix
Signed-off-by: Asim Viladi Oglu Manizada <manizada@pm.me>
Reviewed-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
Link: https://patch.msgid.link/20260706094336.38639-1-manizada@pm.me
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 13052408a132f489954973d910d8430fca05e2ac..d8079dee700edc3881a501355084705bfd4cb42c 100644 (file)
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2496,13 +2496,56 @@ static inline int add_nested_action_start(struct sw_flow_actions **sfa,
        return used;
 }
 
-static inline void add_nested_action_end(struct sw_flow_actions *sfa,
-                                        int st_offset)
+static inline int add_nested_action_end(struct sw_flow_actions *sfa,
+                                       int st_offset)
 {
-       struct nlattr *a = (struct nlattr *) ((unsigned char *)sfa->actions +
-                                                              st_offset);
+       struct nlattr *a;
+       u32 attr_len;
+
+       if (WARN_ON_ONCE(st_offset < 0 ||
+                        (u32)st_offset > sfa->actions_len))
+               return -EINVAL;
+
+       attr_len = sfa->actions_len - (u32)st_offset;
+       if (WARN_ON_ONCE(attr_len < NLA_HDRLEN))
+               return -EINVAL;
 
-       a->nla_len = sfa->actions_len - st_offset;
+       if (attr_len > U16_MAX)
+               return -EMSGSIZE;
+
+       a = (struct nlattr *)((u8 *)sfa->actions + st_offset);
+       a->nla_len = attr_len;
+       return 0;
+}
+
+/* Free the generated action-list tail at @start and truncate it.
+ * If @nested, @start points to its containing nlattr header.
+ */
+static void ovs_nla_trim(struct sw_flow_actions *sfa, int start, bool nested)
+{
+       const struct nlattr *actions;
+       u32 len;
+
+       if (start < 0)
+               return;
+
+       if (WARN_ON_ONCE((u32)start > sfa->actions_len))
+               return;
+
+       actions = (const struct nlattr *)((u8 *)sfa->actions + start);
+       len = sfa->actions_len - (u32)start;
+
+       if (nested) {
+               if (len < NLA_HDRLEN)
+                       goto out;
+
+               actions = (const struct nlattr *)((u8 *)actions + NLA_HDRLEN);
+               len -= NLA_HDRLEN;
+       }
+
+       ovs_nla_free_nested_actions(actions, len);
+out:
+       sfa->actions_len = start;
 }
 
 static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
@@ -2522,6 +2565,7 @@ static int validate_and_copy_sample(struct net *net, const struct nlattr *attr,
        const struct nlattr *attrs[OVS_SAMPLE_ATTR_MAX + 1];
        const struct nlattr *probability, *actions;
        const struct nlattr *a;
+       int actions_start;
        int rem, start, err;
        struct sample_arg arg;
 
@@ -2565,18 +2609,27 @@ static int validate_and_copy_sample(struct net *net, const struct nlattr *attr,
        err = ovs_nla_add_action(sfa, OVS_SAMPLE_ATTR_ARG, &arg, sizeof(arg),
                                 log);
        if (err)
-               return err;
+               goto err;
 
+       actions_start = (*sfa)->actions_len;
        err = __ovs_nla_copy_actions(net, actions, key, sfa,
                                     eth_type, vlan_tci, mpls_label_count, log,
                                     depth + 1);
 
        if (err)
-               return err;
+               goto err_free;
 
-       add_nested_action_end(*sfa, start);
+       err = add_nested_action_end(*sfa, start);
+       if (err)
+               goto err_free;
 
        return 0;
+
+err_free:
+       ovs_nla_trim(*sfa, actions_start, false);
+err:
+       (*sfa)->actions_len = start;
+       return err;
 }
 
 static int validate_and_copy_dec_ttl(struct net *net,
@@ -2624,18 +2677,31 @@ static int validate_and_copy_dec_ttl(struct net *net,
                return start;
 
        action_start = add_nested_action_start(sfa, OVS_DEC_TTL_ATTR_ACTION, log);
-       if (action_start < 0)
-               return action_start;
+       if (action_start < 0) {
+               err = action_start;
+               goto err;
+       }
 
        err = __ovs_nla_copy_actions(net, actions, key, sfa, eth_type,
                                     vlan_tci, mpls_label_count, log,
                                     depth + 1);
        if (err)
-               return err;
+               goto err_free;
+
+       err = add_nested_action_end(*sfa, action_start);
+       if (err)
+               goto err_free;
 
-       add_nested_action_end(*sfa, action_start);
-       add_nested_action_end(*sfa, start);
+       err = add_nested_action_end(*sfa, start);
+       if (err)
+               goto err_free;
        return 0;
+
+err_free:
+       ovs_nla_trim(*sfa, action_start, true);
+err:
+       (*sfa)->actions_len = start;
+       return err;
 }
 
 static int validate_and_copy_clone(struct net *net,
@@ -2646,6 +2712,7 @@ static int validate_and_copy_clone(struct net *net,
                                   u32 mpls_label_count, bool log, bool last,
                                   u32 depth)
 {
+       int actions_start;
        int start, err;
        u32 exec;
 
@@ -2661,17 +2728,26 @@ static int validate_and_copy_clone(struct net *net,
        err = ovs_nla_add_action(sfa, OVS_CLONE_ATTR_EXEC, &exec,
                                 sizeof(exec), log);
        if (err)
-               return err;
+               goto err;
 
+       actions_start = (*sfa)->actions_len;
        err = __ovs_nla_copy_actions(net, attr, key, sfa,
                                     eth_type, vlan_tci, mpls_label_count, log,
                                     depth + 1);
        if (err)
-               return err;
+               goto err_free;
 
-       add_nested_action_end(*sfa, start);
+       err = add_nested_action_end(*sfa, start);
+       if (err)
+               goto err_free;
 
        return 0;
+
+err_free:
+       ovs_nla_trim(*sfa, actions_start, false);
+err:
+       (*sfa)->actions_len = start;
+       return err;
 }
 
 void ovs_match_init(struct sw_flow_match *match,
@@ -2763,20 +2839,20 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
        tun_dst = metadata_dst_alloc(key.tun_opts_len, METADATA_IP_TUNNEL,
                                     GFP_KERNEL);
 
-       if (!tun_dst)
-               return -ENOMEM;
+       if (!tun_dst) {
+               err = -ENOMEM;
+               goto err;
+       }
 
        err = dst_cache_init(&tun_dst->u.tun_info.dst_cache, GFP_KERNEL);
-       if (err) {
-               dst_release((struct dst_entry *)tun_dst);
-               return err;
-       }
+       if (err)
+               goto err_free_tun_dst;
 
        a = __add_action(sfa, OVS_KEY_ATTR_TUNNEL_INFO, NULL,
                         sizeof(*ovs_tun), log);
        if (IS_ERR(a)) {
-               dst_release((struct dst_entry *)tun_dst);
-               return PTR_ERR(a);
+               err = PTR_ERR(a);
+               goto err_free_tun_dst;
        }
 
        ovs_tun = nla_data(a);
@@ -2797,8 +2873,16 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
        ip_tunnel_info_opts_set(tun_info,
                                TUN_METADATA_OPTS(&key, key.tun_opts_len),
                                key.tun_opts_len, dst_opt_type);
-       add_nested_action_end(*sfa, start);
+       err = add_nested_action_end(*sfa, start);
+       if (WARN_ON_ONCE(err))
+               goto err_free_tun_dst;
+
+       return 0;
 
+err_free_tun_dst:
+       dst_release((struct dst_entry *)tun_dst);
+err:
+       (*sfa)->actions_len = start;
        return err;
 }
 
@@ -2971,7 +3055,7 @@ static int validate_set(const struct nlattr *a,
 
        /* Convert non-masked non-tunnel set actions to masked set actions. */
        if (!masked && key_type != OVS_KEY_ATTR_TUNNEL) {
-               int start, len = key_len * 2;
+               int err, start, len = key_len * 2;
                struct nlattr *at;
 
                *skip_copy = true;
@@ -2983,8 +3067,11 @@ static int validate_set(const struct nlattr *a,
                        return start;
 
                at = __add_action(sfa, key_type, NULL, len, log);
-               if (IS_ERR(at))
-                       return PTR_ERR(at);
+               if (IS_ERR(at)) {
+                       err = PTR_ERR(at);
+                       (*sfa)->actions_len = start;
+                       return err;
+               }
 
                memcpy(nla_data(at), nla_data(ovs_key), key_len); /* Key. */
                memset(nla_data(at) + key_len, 0xff, key_len);    /* Mask. */
@@ -2994,7 +3081,11 @@ static int validate_set(const struct nlattr *a,
 
                        mask->ipv6_label &= htonl(0x000FFFFF);
                }
-               add_nested_action_end(*sfa, start);
+               err = add_nested_action_end(*sfa, start);
+               if (WARN_ON_ONCE(err)) {
+                       (*sfa)->actions_len = start;
+                       return err;
+               }
        }
 
        return 0;
@@ -3040,7 +3131,8 @@ static int validate_and_copy_check_pkt_len(struct net *net,
        const struct nlattr *acts_if_greater, *acts_if_lesser_eq;
        struct nlattr *a[OVS_CHECK_PKT_LEN_ATTR_MAX + 1];
        struct check_pkt_len_arg arg;
-       int nested_acts_start;
+       int greater_acts_start = -1;
+       int lesser_acts_start = -1;
        int start, err;
 
        err = nla_parse_deprecated_strict(a, OVS_CHECK_PKT_LEN_ATTR_MAX,
@@ -3075,37 +3167,58 @@ static int validate_and_copy_check_pkt_len(struct net *net,
        err = ovs_nla_add_action(sfa, OVS_CHECK_PKT_LEN_ATTR_ARG, &arg,
                                 sizeof(arg), log);
        if (err)
-               return err;
+               goto err_free;
 
-       nested_acts_start = add_nested_action_start(sfa,
-               OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL, log);
-       if (nested_acts_start < 0)
-               return nested_acts_start;
+       lesser_acts_start =
+               add_nested_action_start(sfa,
+                                       OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL,
+                                       log);
+       if (lesser_acts_start < 0) {
+               err = lesser_acts_start;
+               goto err_free;
+       }
 
        err = __ovs_nla_copy_actions(net, acts_if_lesser_eq, key, sfa,
                                     eth_type, vlan_tci, mpls_label_count, log,
                                     depth + 1);
 
        if (err)
-               return err;
+               goto err_free;
 
-       add_nested_action_end(*sfa, nested_acts_start);
+       err = add_nested_action_end(*sfa, lesser_acts_start);
+       if (err)
+               goto err_free;
 
-       nested_acts_start = add_nested_action_start(sfa,
-               OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER, log);
-       if (nested_acts_start < 0)
-               return nested_acts_start;
+       greater_acts_start =
+               add_nested_action_start(sfa,
+                                       OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER,
+                                       log);
+       if (greater_acts_start < 0) {
+               err = greater_acts_start;
+               goto err_free;
+       }
 
        err = __ovs_nla_copy_actions(net, acts_if_greater, key, sfa,
                                     eth_type, vlan_tci, mpls_label_count, log,
                                     depth + 1);
 
        if (err)
-               return err;
+               goto err_free;
+
+       err = add_nested_action_end(*sfa, greater_acts_start);
+       if (err)
+               goto err_free;
 
-       add_nested_action_end(*sfa, nested_acts_start);
-       add_nested_action_end(*sfa, start);
+       err = add_nested_action_end(*sfa, start);
+       if (err)
+               goto err_free;
        return 0;
+
+err_free:
+       ovs_nla_trim(*sfa, greater_acts_start, true);
+       ovs_nla_trim(*sfa, lesser_acts_start, true);
+       ovs_nla_trim(*sfa, start, false);
+       return err;
 }
 
 static int validate_psample(const struct nlattr *attr)