Thread (5 messages) 5 messages, 5 authors, 2d ago

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

From: Ilya Maximets <i.maximets@ovn.org>
Date: 2026-07-06 13:16:20
Also in: stable

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 <redacted>
---
 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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help