On 23-2-2016 21:52, Johannes Berg wrote:

On Tue, 2016-02-23 at 21:46 +0100, Arend Van Spriel wrote:

quoted

quoted

Maybe we should be less specific here regarding the NLA_FLAG and
say
that it will contain attributes for each, indicating which
behaviours
are supported, and say there's behaviour-dependent data inside each
of
those attributes?

For the GET_WIPHY case there will be not behaviour dependent data.

quoted

It seems entirely plausible that future behaviours would require
their
own sub-capabilities; perhaps that's even the case today for having
an
adjustment range for example, not that I think it's really
necessary
now.

At least it does not harm to take that into account.

Right; there isn't today, but there might be later, so for the
GET_WIPHY case it might be worth just specifying that there will be an
attribute, not that it's NLA_FLAG?

Will try and be vague :-)

Not that it'll matter much - only when we actually add such could
applications possibly want to parse them :)

That goes without saying :-p

quoted

quoted

quoted

+	/* only process one nested attribute */
+	nest = nla_data(nla);
+	if (!nla_ok(nest, nla_len(nest)))
+		return -EBADF;

This isn't quite clear to me. Shouldn't you do this by declaring it
NLA_TESTED in the nl80211_policy?

Guess you mean NLA_NESTED.

Heh, yeah.

quoted

quoted

Actually, not really? you use nla_ok() on the inner (without having
checked at all that it's even long enough btw, since you didn't do
the
policy change), but that would already be done by nla_parse()
below?

Because ATTR_BSS_SELECT is used in CMD_GET_WIPHY and CMD_CONNECT the
length of the attribute is not a constant. Or do I only need to put
length in nl80211_policy for userspace-2-kernel direction, ie. for
ATTR_BSS_SELECT in CMD_CONNECT.

Ah. kernel->userspace messages don't use the policy in any way, it's
just for the userspace->kernel message validation, so you can put there
what's needed for CMD_CONNECT and ignore GET_WIPHY.

I actually do not see any NLA_NESTED attributes with an explicit length.
As you mentioned the nla_parse() of the nested attribute will validate
the length of the stream so no need to put that in the policy.

Regards,
Arend