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: [email protected]
> Assisted-by: avom-custom-harness:gpt-5.5-qwen3.6-mod-mix
> Signed-off-by: Asim Viladi Oglu Manizada <[email protected]>
> ---
> 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 <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
