| From 726f2f53f19e684d96f7e9e0779d07bfeba66f34 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 14 Aug 2023 13:38:40 -0700 |
| Subject: net: openvswitch: reject negative ifindex |
| |
| From: Jakub Kicinski <kuba@kernel.org> |
| |
| [ Upstream commit a552bfa16bab4ce901ee721346a28c4e483f4066 ] |
| |
| Recent changes in net-next (commit 759ab1edb56c ("net: store netdevs |
| in an xarray")) refactored the handling of pre-assigned ifindexes |
| and let syzbot surface a latent problem in ovs. ovs does not validate |
| ifindex, making it possible to create netdev ports with negative |
| ifindex values. It's easy to repro with YNL: |
| |
| $ ./cli.py --spec netlink/specs/ovs_datapath.yaml \ |
| --do new \ |
| --json '{"upcall-pid": 1, "name":"my-dp"}' |
| $ ./cli.py --spec netlink/specs/ovs_vport.yaml \ |
| --do new \ |
| --json '{"upcall-pid": "00000001", "name": "some-port0", "dp-ifindex":3,"ifindex":4294901760,"type":2}' |
| |
| $ ip link show |
| -65536: some-port0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000 |
| link/ether 7a:48:21:ad:0b:fb brd ff:ff:ff:ff:ff:ff |
| ... |
| |
| Validate the inputs. Now the second command correctly returns: |
| |
| $ ./cli.py --spec netlink/specs/ovs_vport.yaml \ |
| --do new \ |
| --json '{"upcall-pid": "00000001", "name": "some-port0", "dp-ifindex":3,"ifindex":4294901760,"type":2}' |
| |
| lib.ynl.NlError: Netlink error: Numerical result out of range |
| nl_len = 108 (92) nl_flags = 0x300 nl_type = 2 |
| error: -34 extack: {'msg': 'integer out of range', 'unknown': [[type:4 len:36] b'\x0c\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0c\x00\x03\x00\xff\xff\xff\x7f\x00\x00\x00\x00\x08\x00\x01\x00\x08\x00\x00\x00'], 'bad-attr': '.ifindex'} |
| |
| Accept 0 since it used to be silently ignored. |
| |
| Fixes: 54c4ef34c4b6 ("openvswitch: allow specifying ifindex of new interfaces") |
| Reported-by: syzbot+7456b5dcf65111553320@syzkaller.appspotmail.com |
| Reviewed-by: Leon Romanovsky <leonro@nvidia.com> |
| Reviewed-by: Aaron Conole <aconole@redhat.com> |
| Link: https://lore.kernel.org/r/20230814203840.2908710-1-kuba@kernel.org |
| Signed-off-by: Jakub Kicinski <kuba@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/openvswitch/datapath.c | 8 ++++---- |
| 1 file changed, 4 insertions(+), 4 deletions(-) |
| |
| diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c |
| index a6d2a0b1aa21e..3d7a91e64c88f 100644 |
| --- a/net/openvswitch/datapath.c |
| +++ b/net/openvswitch/datapath.c |
| @@ -1829,7 +1829,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info) |
| parms.port_no = OVSP_LOCAL; |
| parms.upcall_portids = a[OVS_DP_ATTR_UPCALL_PID]; |
| parms.desired_ifindex = a[OVS_DP_ATTR_IFINDEX] |
| - ? nla_get_u32(a[OVS_DP_ATTR_IFINDEX]) : 0; |
| + ? nla_get_s32(a[OVS_DP_ATTR_IFINDEX]) : 0; |
| |
| /* So far only local changes have been made, now need the lock. */ |
| ovs_lock(); |
| @@ -2049,7 +2049,7 @@ static const struct nla_policy datapath_policy[OVS_DP_ATTR_MAX + 1] = { |
| [OVS_DP_ATTR_USER_FEATURES] = { .type = NLA_U32 }, |
| [OVS_DP_ATTR_MASKS_CACHE_SIZE] = NLA_POLICY_RANGE(NLA_U32, 0, |
| PCPU_MIN_UNIT_SIZE / sizeof(struct mask_cache_entry)), |
| - [OVS_DP_ATTR_IFINDEX] = {.type = NLA_U32 }, |
| + [OVS_DP_ATTR_IFINDEX] = NLA_POLICY_MIN(NLA_S32, 0), |
| }; |
| |
| static const struct genl_small_ops dp_datapath_genl_ops[] = { |
| @@ -2302,7 +2302,7 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info) |
| parms.port_no = port_no; |
| parms.upcall_portids = a[OVS_VPORT_ATTR_UPCALL_PID]; |
| parms.desired_ifindex = a[OVS_VPORT_ATTR_IFINDEX] |
| - ? nla_get_u32(a[OVS_VPORT_ATTR_IFINDEX]) : 0; |
| + ? nla_get_s32(a[OVS_VPORT_ATTR_IFINDEX]) : 0; |
| |
| vport = new_vport(&parms); |
| err = PTR_ERR(vport); |
| @@ -2539,7 +2539,7 @@ static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = { |
| [OVS_VPORT_ATTR_TYPE] = { .type = NLA_U32 }, |
| [OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_UNSPEC }, |
| [OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED }, |
| - [OVS_VPORT_ATTR_IFINDEX] = { .type = NLA_U32 }, |
| + [OVS_VPORT_ATTR_IFINDEX] = NLA_POLICY_MIN(NLA_S32, 0), |
| [OVS_VPORT_ATTR_NETNSID] = { .type = NLA_S32 }, |
| [OVS_VPORT_ATTR_UPCALL_STATS] = { .type = NLA_NESTED }, |
| }; |
| -- |
| 2.40.1 |
| |