On 23-2-2016 21:52, Johannes Berg wrote: > On Tue, 2016-02-23 at 21:46 +0100, Arend Van Spriel wrote: >> >>> 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. >> >>> 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 >>>> + /* 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. > >>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html