Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net: openvswitch: fix middle attribute validation in push_nsh() action

The push_nsh() action structure looks like this:

OVS_ACTION_ATTR_PUSH_NSH(OVS_KEY_ATTR_NSH(OVS_NSH_KEY_ATTR_BASE,...))

The outermost OVS_ACTION_ATTR_PUSH_NSH attribute is OK'ed by the
nla_for_each_nested() inside __ovs_nla_copy_actions(). The innermost
OVS_NSH_KEY_ATTR_BASE/MD1/MD2 are OK'ed by the nla_for_each_nested()
inside nsh_key_put_from_nlattr(). But nothing checks if the attribute
in the middle is OK. We don't even check that this attribute is the
OVS_KEY_ATTR_NSH. We just do a double unwrap with a pair of nla_data()
calls - first time directly while calling validate_push_nsh() and the
second time as part of the nla_for_each_nested() macro, which isn't
safe, potentially causing invalid memory access if the size of this
attribute is incorrect. The failure may not be noticed during
validation due to larger netlink buffer, but cause trouble later during
action execution where the buffer is allocated exactly to the size:

BUG: KASAN: slab-out-of-bounds in nsh_hdr_from_nlattr+0x1dd/0x6a0 [openvswitch]
Read of size 184 at addr ffff88816459a634 by task a.out/22624

CPU: 8 UID: 0 PID: 22624 6.18.0-rc7+ #115 PREEMPT(voluntary)
Call Trace:
<TASK>
dump_stack_lvl+0x51/0x70
print_address_description.constprop.0+0x2c/0x390
kasan_report+0xdd/0x110
kasan_check_range+0x35/0x1b0
__asan_memcpy+0x20/0x60
nsh_hdr_from_nlattr+0x1dd/0x6a0 [openvswitch]
push_nsh+0x82/0x120 [openvswitch]
do_execute_actions+0x1405/0x2840 [openvswitch]
ovs_execute_actions+0xd5/0x3b0 [openvswitch]
ovs_packet_cmd_execute+0x949/0xdb0 [openvswitch]
genl_family_rcv_msg_doit+0x1d6/0x2b0
genl_family_rcv_msg+0x336/0x580
genl_rcv_msg+0x9f/0x130
netlink_rcv_skb+0x11f/0x370
genl_rcv+0x24/0x40
netlink_unicast+0x73e/0xaa0
netlink_sendmsg+0x744/0xbf0
__sys_sendto+0x3d6/0x450
do_syscall_64+0x79/0x2c0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
</TASK>

Let's add some checks that the attribute is properly sized and it's
the only one attribute inside the action. Technically, there is no
real reason for OVS_KEY_ATTR_NSH to be there, as we know that we're
pushing an NSH header already, it just creates extra nesting, but
that's how uAPI works today. So, keeping as it is.

Fixes: b2d0f5d5dc53 ("openvswitch: enable NSH support")
Reported-by: Junvy Yang <zhuque@tencent.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Eelco Chaudron echaudro@redhat.com
Reviewed-by: Aaron Conole <aconole@redhat.com>
Link: https://patch.msgid.link/20251204105334.900379-1-i.maximets@ovn.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Ilya Maximets and committed by
Jakub Kicinski
5ace7ef8 186468c6

+10 -3
+10 -3
net/openvswitch/flow_netlink.c
··· 2802 2802 return err; 2803 2803 } 2804 2804 2805 - static bool validate_push_nsh(const struct nlattr *attr, bool log) 2805 + static bool validate_push_nsh(const struct nlattr *a, bool log) 2806 2806 { 2807 + struct nlattr *nsh_key = nla_data(a); 2807 2808 struct sw_flow_match match; 2808 2809 struct sw_flow_key key; 2809 2810 2811 + /* There must be one and only one NSH header. */ 2812 + if (!nla_ok(nsh_key, nla_len(a)) || 2813 + nla_total_size(nla_len(nsh_key)) != nla_len(a) || 2814 + nla_type(nsh_key) != OVS_KEY_ATTR_NSH) 2815 + return false; 2816 + 2810 2817 ovs_match_init(&match, &key, true, NULL); 2811 - return !nsh_key_put_from_nlattr(attr, &match, false, true, log); 2818 + return !nsh_key_put_from_nlattr(nsh_key, &match, false, true, log); 2812 2819 } 2813 2820 2814 2821 /* Return false if there are any non-masked bits set. ··· 3396 3389 return -EINVAL; 3397 3390 } 3398 3391 mac_proto = MAC_PROTO_NONE; 3399 - if (!validate_push_nsh(nla_data(a), log)) 3392 + if (!validate_push_nsh(a, log)) 3400 3393 return -EINVAL; 3401 3394 break; 3402 3395