| From b41f0a0de980133f34a43f178e48a0baf2bfeac3 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Thu, 4 Dec 2025 11:53:32 +0100 |
| Subject: net: openvswitch: fix middle attribute validation in push_nsh() |
| action |
| |
| From: Ilya Maximets <i.maximets@ovn.org> |
| |
| [ Upstream commit 5ace7ef87f059d68b5f50837ef3e8a1a4870c36e ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/openvswitch/flow_netlink.c | 13 ++++++++++--- |
| 1 file changed, 10 insertions(+), 3 deletions(-) |
| |
| diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c |
| index 7c2692f897f99..a7a9e4df3f600 100644 |
| --- a/net/openvswitch/flow_netlink.c |
| +++ b/net/openvswitch/flow_netlink.c |
| @@ -2757,13 +2757,20 @@ static int validate_and_copy_set_tun(const struct nlattr *attr, |
| return err; |
| } |
| |
| -static bool validate_push_nsh(const struct nlattr *attr, bool log) |
| +static bool validate_push_nsh(const struct nlattr *a, bool log) |
| { |
| + struct nlattr *nsh_key = nla_data(a); |
| struct sw_flow_match match; |
| struct sw_flow_key key; |
| |
| + /* There must be one and only one NSH header. */ |
| + if (!nla_ok(nsh_key, nla_len(a)) || |
| + nla_total_size(nla_len(nsh_key)) != nla_len(a) || |
| + nla_type(nsh_key) != OVS_KEY_ATTR_NSH) |
| + return false; |
| + |
| ovs_match_init(&match, &key, true, NULL); |
| - return !nsh_key_put_from_nlattr(attr, &match, false, true, log); |
| + return !nsh_key_put_from_nlattr(nsh_key, &match, false, true, log); |
| } |
| |
| /* Return false if there are any non-masked bits set. |
| @@ -3317,7 +3324,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, |
| return -EINVAL; |
| } |
| mac_proto = MAC_PROTO_NONE; |
| - if (!validate_push_nsh(nla_data(a), log)) |
| + if (!validate_push_nsh(a, log)) |
| return -EINVAL; |
| break; |
| |
| -- |
| 2.51.0 |
| |