| Message ID | 3b6fee130e2d264242463cff063bcfb6d6f5da83.1637494779.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [mac80211] cfg80211: check nla_parse_nested return code in nl80211_set_sar_specs | expand |
On Sun, 2021-11-21 at 12:41 +0100, Lorenzo Bianconi wrote: > Check error code returned by nla_parse_nested in nl80211_set_sar_specs > routine before parsing SAR sub-specs. > I don't think we need to. The policy already states it: [NL80211_ATTR_SAR_SPEC] = NLA_POLICY_NESTED(sar_policy), Since we always parse the top-level already in the generic netlink code, it will recurse down here by way of that policy link, and then it will already be validated as much as nla_parse_nested() can do again - so we only do it again here to actually get access to the pointers. johannes
> On Sun, 2021-11-21 at 12:41 +0100, Lorenzo Bianconi wrote: > > Check error code returned by nla_parse_nested in nl80211_set_sar_specs > > routine before parsing SAR sub-specs. > > > > I don't think we need to. The policy already states it: > > [NL80211_ATTR_SAR_SPEC] = NLA_POLICY_NESTED(sar_policy), ack, right, I missed that. I guess we can drop the patch. Regards, Lorenzo > > > Since we always parse the top-level already in the generic netlink code, > it will recurse down here by way of that policy link, and then it will > already be validated as much as nla_parse_nested() can do again - so we > only do it again here to actually get access to the pointers. > > johannes >
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index a27b3b5fa210..c2b005d0d29a 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -15294,9 +15294,11 @@ static int nl80211_set_sar_specs(struct sk_buff *skb, struct genl_info *info) if (!info->attrs[NL80211_ATTR_SAR_SPEC]) return -EINVAL; - nla_parse_nested(tb, NL80211_SAR_ATTR_MAX, - info->attrs[NL80211_ATTR_SAR_SPEC], - NULL, NULL); + err = nla_parse_nested(tb, NL80211_SAR_ATTR_MAX, + info->attrs[NL80211_ATTR_SAR_SPEC], + NULL, NULL); + if (err) + return err; if (!tb[NL80211_SAR_ATTR_TYPE] || !tb[NL80211_SAR_ATTR_SPECS]) return -EINVAL; @@ -15319,16 +15321,17 @@ static int nl80211_set_sar_specs(struct sk_buff *skb, struct genl_info *info) sar_spec->type = type; specs = 0; nla_for_each_nested(spec_list, tb[NL80211_SAR_ATTR_SPECS], rem) { - nla_parse_nested(spec, NL80211_SAR_ATTR_SPECS_MAX, - spec_list, NULL, NULL); + err = nla_parse_nested(spec, NL80211_SAR_ATTR_SPECS_MAX, + spec_list, NULL, NULL); + if (err) + goto error; switch (type) { case NL80211_SAR_TYPE_POWER: - if (nl80211_set_sar_sub_specs(rdev, sar_spec, - spec, specs)) { - err = -EINVAL; + err = nl80211_set_sar_sub_specs(rdev, sar_spec, spec, + specs); + if (err) goto error; - } break; default: err = -EINVAL;
Check error code returned by nla_parse_nested in nl80211_set_sar_specs routine before parsing SAR sub-specs. Fixes: 6bdb68cef7bf5 ("nl80211: add common API to configure SAR power limitations") Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- net/wireless/nl80211.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)