From 956db0a13b47df7f3d6d624394e602e8bf9b057e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 14 Aug 2023 13:56:25 -0700 Subject: [PATCH 1/3] net: warn about attempts to register negative ifindex Since the xarray changes we mix returning valid ifindex and negative errno in a single int returned from dev_index_reserve(). This depends on the fact that ifindexes can't be negative. Otherwise we may insert into the xarray and return a very large negative value. This in turn may break ERR_PTR(). OvS is susceptible to this problem and lacking validation (fix posted separately for net). Reject negative ifindex explicitly. Add a warning because the input validation is better handled by the caller. Reviewed-by: Leon Romanovsky Link: https://lore.kernel.org/r/20230814205627.2914583-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/core/dev.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/net/core/dev.c b/net/core/dev.c index 636b41f0b32d..17e6281e408c 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -9589,6 +9589,11 @@ static int dev_index_reserve(struct net *net, u32 ifindex) { int err; + if (ifindex > INT_MAX) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EINVAL; + } + if (!ifindex) err = xa_alloc_cyclic(&net->dev_by_index, &ifindex, NULL, xa_limit_31b, &net->ifindex, GFP_KERNEL); From ded67d90815a412f327051d27eb6c6de57cd2c03 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 14 Aug 2023 13:56:26 -0700 Subject: [PATCH 2/3] netlink: specs: add ovs_vport new command Add NEW to the spec, it was useful testing the fix for OvS input validation. Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20230814205627.2914583-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- Documentation/netlink/specs/ovs_vport.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Documentation/netlink/specs/ovs_vport.yaml b/Documentation/netlink/specs/ovs_vport.yaml index 17336455bec1..ef298b001445 100644 --- a/Documentation/netlink/specs/ovs_vport.yaml +++ b/Documentation/netlink/specs/ovs_vport.yaml @@ -81,6 +81,10 @@ attribute-sets: name-prefix: ovs-vport-attr- enum-name: ovs-vport-attr attributes: + - + name: unspec + type: unused + value: 0 - name: port-no type: u32 @@ -120,6 +124,20 @@ attribute-sets: operations: name-prefix: ovs-vport-cmd- list: + - + name: new + doc: Create a new OVS vport + attribute-set: vport + fixed-header: ovs-header + do: + request: + attributes: + - name + - type + - upcall-pid + - dp-ifindex + - ifindex + - options - name: get doc: Get / dump OVS vport configuration and state From 7582113c6917c280c90352d1935cfa451e74376a Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 14 Aug 2023 13:56:27 -0700 Subject: [PATCH 3/3] tools: ynl: add more info to KeyErrors on missing attrs When developing specs its useful to know which attr space YNL was trying to find an attribute in on key error. Instead of printing: KeyError: 0 add info about the space: Exception: Space 'vport' has no attribute with value '0' Reviewed-by: Donald Hunter Link: https://lore.kernel.org/r/20230814205627.2914583-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index 3ca28d4bcb18..6951bcc7efdc 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -395,7 +395,10 @@ class YnlFamily(SpecFamily): self.family.genl_family['mcast'][mcast_name]) def _add_attr(self, space, name, value): - attr = self.attr_sets[space][name] + try: + attr = self.attr_sets[space][name] + except KeyError: + raise Exception(f"Space '{space}' has no attribute '{name}'") nl_type = attr.value if attr["type"] == 'nest': nl_type |= Netlink.NLA_F_NESTED @@ -450,7 +453,10 @@ class YnlFamily(SpecFamily): attr_space = self.attr_sets[space] rsp = dict() for attr in attrs: - attr_spec = attr_space.attrs_by_val[attr.type] + try: + attr_spec = attr_space.attrs_by_val[attr.type] + except KeyError: + raise Exception(f"Space '{space}' has no attribute with value '{attr.type}'") if attr_spec["type"] == 'nest': subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes']) decoded = subdict @@ -479,7 +485,10 @@ class YnlFamily(SpecFamily): def _decode_extack_path(self, attrs, attr_set, offset, target): for attr in attrs: - attr_spec = attr_set.attrs_by_val[attr.type] + try: + attr_spec = attr_set.attrs_by_val[attr.type] + except KeyError: + raise Exception(f"Space '{attr_set.name}' has no attribute with value '{attr.type}'") if offset > target: break if offset == target: