netdev - Re: [PATCH v8 1/5] mac80211_hwsim: add PMSR capability support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51c2b615d848c227edae52cc07df334695c7f856.camel@sipsolutions.net>
Date:   Wed, 08 Mar 2023 09:06:51 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     Jaewan Kim <jaewan@...gle.com>,
        Simon Horman <simon.horman@...igine.com>
Cc:     gregkh@...uxfoundation.org, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, kernel-team@...roid.com, adelva@...gle.com
Subject: Re: [PATCH v8 1/5] mac80211_hwsim: add PMSR capability support

On Wed, 2023-03-08 at 08:00 +0000, Jaewan Kim wrote:
> > 
> > > +static int parse_pmsr_capa(const struct nlattr *pmsr_capa, struct cfg80211_pmsr_capabilities *out,
> > > +                        struct genl_info *info)
> > > +{
> > > +     struct nlattr *tb[NL80211_PMSR_ATTR_MAX + 1];
> > > +     struct nlattr *nla;
> > > +     int size;
> >  +      int ret = nla_parse_nested(tb, NL80211_PMSR_ATTR_MAX, pmsr_capa,
> > > +                                hwsim_pmsr_capa_policy, NULL);
> > > +
> > > +     if (ret) {
> > > +             NL_SET_ERR_MSG_ATTR(info->extack, pmsr_capa, "malformed PMSR capability");
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     if (tb[NL80211_PMSR_ATTR_MAX_PEERS])
> > > +             out->max_peers = nla_get_u32(tb[NL80211_PMSR_ATTR_MAX_PEERS]);
> > > +     out->report_ap_tsf = !!tb[NL80211_PMSR_ATTR_REPORT_AP_TSF];
> > > +     out->randomize_mac_addr = !!tb[NL80211_PMSR_ATTR_RANDOMIZE_MAC_ADDR];
> > > +
> > > +     if (!tb[NL80211_PMSR_ATTR_TYPE_CAPA]) {
> > > +             NL_SET_ERR_MSG_ATTR(info->extack, tb[NL80211_PMSR_ATTR_TYPE_CAPA],
> > > +                                 "malformed PMSR type");
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     nla_for_each_nested(nla, tb[NL80211_PMSR_ATTR_TYPE_CAPA], size) {
> > > +             switch (nla_type(nla)) {
> > > +             case NL80211_PMSR_TYPE_FTM:
> > > +                     parse_ftm_capa(nla, out, info);
> > > +                     break;
> > > +             default:
> > > +                     WARN_ON(1);
> > 
> > WARN_ON doesn't seem right here. I suspect that the following is more fitting.
> > 
> >                 NL_SET_ERR_MSG_ATTR(...);
> >                 return -EINVAL;
> > 
> 
> Not using NL_SET_ERR_MSG_ATTR(...) is intended to follow the pattern
> of net/wireless/pmsr.c,
> where unknown type isn't considered as an error.

NL80211_PMSR_ATTR_TYPE_CAPA is normally NLA_REJECT (not sent by
userspace), you just use it here for the hwsim capabilities which makes
sense, but it feels better to just reject unknown types.

If you're thinking of actually using it we still have in pmsr.c this
code:

        nla_for_each_nested(treq, req[NL80211_PMSR_REQ_ATTR_DATA], rem) {
                switch (nla_type(treq)) {
                case NL80211_PMSR_TYPE_FTM:
                        err = pmsr_parse_ftm(rdev, treq, out, info);
                        break;
                default:
                        NL_SET_ERR_MSG_ATTR(info->extack, treq,
                                            "unsupported measurement type");
                        err = -EINVAL;
                }


johannes