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] [day] [month] [year] [list]

Message-ID: <CABZjns4feHMHtoR5+AYBRpP-Jo=BMaG4Wa2UF2ouWoKFQeuuBg@mail.gmail.com>
Date:   Mon, 13 Mar 2023 16:54:48 +0900
From:   Jaewan Kim <jaewan@...gle.com>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     Simon Horman <simon.horman@...igine.com>,
        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, Mar 8, 2023 at 5:07 PM Johannes Berg <johannes@...solutions.net> wrote:
>
> 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

Done, and uploaded the next patchset for this. Thank you for the review.

-- 
Jaewan Kim (김재완) | Software Engineer in Google Korea |
jaewan@...gle.com | +82-10-2781-5078