Re: [PATCH net] net: openvswitch: reject oversized nested action attrs

[Ilya Maximets <i.maximets@ovn.org>]

On 7/6/26 11:44 AM, Asim Viladi Oglu Manizada wrote:
> 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>
> ---
>  net/openvswitch/flow_netlink.c | 201 +++++++++++++++++++++++++--------
>  1 file changed, 157 insertions(+), 44 deletions(-)

Thanks!  As I said before, this is a bit of a lengthy fix, but it seems
to be the best way of dealing with this whole class of issues at once,
given the restrictions of the historical interface.

I see checkpatch complains about couple lines being over 80, but it's
hard to please it with such a long enum names, so it's fine.  The
'inline' thing is also pre-existing and unrelated to the change.

Let's see if LLMs will find anything (they didn't when I tried locally),
but the change LGTM.

Reviewed-by: Ilya Maximets <i.maximets@ovn.org>