The vport netlink reply helpers allocate a fixed-size skb with
nlmsg_new(NLMSG_DEFAULT_SIZE, ...) but serialize the full upcall PID
array via ovs_vport_get_upcall_portids(). Since
ovs_vport_set_upcall_portids() accepts any non-zero multiple of
sizeof(u32) with no upper bound, a CAP_NET_ADMIN user can install a PID
array large enough to overflow the reply buffer, causing nla_put() to
fail with -EMSGSIZE and hitting BUG_ON(err < 0). On systems with
unprivileged user namespaces enabled (e.g., Ubuntu default), this is
reachable via unshare -Urn since OVS vport mutation operations use
GENL_UNS_ADMIN_PERM.
kernel BUG at net/openvswitch/datapath.c:2414!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
CPU: 1 UID: 0 PID: 65 Comm: poc Not tainted 7.0.0-rc7-00195-geb216e422044 #1
RIP: 0010:ovs_vport_cmd_set+0x34c/0x400
Call Trace:
<TASK>
genl_family_rcv_msg_doit (net/netlink/genetlink.c:1116)
genl_rcv_msg (net/netlink/genetlink.c:1194)
netlink_rcv_skb (net/netlink/af_netlink.c:2550)
genl_rcv (net/netlink/genetlink.c:1219)
netlink_unicast (net/netlink/af_netlink.c:1344)
netlink_sendmsg (net/netlink/af_netlink.c:1894)
__sys_sendto (net/socket.c:2206)
__x64_sys_sendto (net/socket.c:2209)
do_syscall_64 (arch/x86/entry/syscall_64.c:63)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
</TASK>
Kernel panic - not syncing: Fatal exception
Reject attempts to set more PIDs than num_possible_cpus() in
ovs_vport_set_upcall_portids(), and pre-compute the worst-case reply
size in ovs_vport_cmd_msg_size() based on that bound, similar to the
existing ovs_dp_cmd_msg_size().
Fixes: 5cd667b0a456 ("openvswitch: Allow each vport to have an array of
'port_id's.")
Reported-by: Xiang Mei <[email protected]>
Signed-off-by: Weiming Shi <[email protected]>
---
v3:
- Cap PID array at num_possible_cpus() in ovs_vport_set_upcall_portids().
- Add ovs_vport_cmd_msg_size() for worst-case reply allocation.
- Keep BUG_ON()s, fix Fixes tag.
v2:
- Dynamically size reply skb instead of using fixed NLMSG_DEFAULT_SIZE.
- Drop WARN_ON_ONCE; use plain error returns instead.
net/openvswitch/datapath.c | 23 +++++++++++++++++++++--
net/openvswitch/vport.c | 3 +++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index e209099218b4..4049bfa1c4df 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -2184,9 +2184,28 @@ static int ovs_vport_cmd_fill_info(struct vport *vport,
struct sk_buff *skb,
return err;
}
+static size_t ovs_vport_cmd_msg_size(void)
+{
+ size_t msgsize = NLMSG_ALIGN(sizeof(struct ovs_header));
+
+ msgsize += nla_total_size(sizeof(u32)); /* OVS_VPORT_ATTR_PORT_NO */
+ msgsize += nla_total_size(sizeof(u32)); /* OVS_VPORT_ATTR_TYPE */
+ msgsize += nla_total_size(IFNAMSIZ);
+ msgsize += nla_total_size(sizeof(u32)); /* OVS_VPORT_ATTR_IFINDEX */
+ msgsize += nla_total_size(sizeof(s32)); /* OVS_VPORT_ATTR_NETNSID */
+ msgsize += nla_total_size_64bit(sizeof(struct ovs_vport_stats));
+ msgsize += nla_total_size(nla_total_size_64bit(sizeof(u64)) +
+ nla_total_size_64bit(sizeof(u64)));
+ msgsize += nla_total_size(num_possible_cpus() * sizeof(u32));
+ msgsize += nla_total_size(nla_total_size(sizeof(u16)) +
+ nla_total_size(nla_total_size(0)));
+
+ return msgsize;
+}
+
static struct sk_buff *ovs_vport_cmd_alloc_info(void)
{
- return nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ return genlmsg_new(ovs_vport_cmd_msg_size(), GFP_KERNEL);
}
/* Called with ovs_mutex, only via ovs_dp_notify_wq(). */
@@ -2196,7 +2215,7 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport
*vport, struct net *net,
struct sk_buff *skb;
int retval;
- skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ skb = ovs_vport_cmd_alloc_info();
if (!skb)
return ERR_PTR(-ENOMEM);
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 23f629e94a36..ccd43bc47bc6 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -406,6 +406,9 @@ int ovs_vport_set_upcall_portids(struct vport *vport, const
struct nlattr *ids)
if (!nla_len(ids) || nla_len(ids) % sizeof(u32))
return -EINVAL;
+ if (nla_len(ids) / sizeof(u32) > num_possible_cpus())
+ return -EINVAL;
+
old = ovsl_dereference(vport->upcall_portids);
vport_portids = kmalloc(sizeof(*vport_portids) + nla_len(ids),
--
2.43.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
