On Wed, 2018-09-12 at 11:15 -0700, David Miller wrote: > This looks great, no objections to this idea or the facility. Great. I'll post this (with the fixups) for real tomorrow then, I guess. A bit too late for me to do now. > It does, however, remind me about about the classic problem of how bad > we are at feature support detection because unrecognized attributes are > ignored. > > I do really hope we can fully solve that problem some day. Yes. There may be two or more levels to this. It wouldn't be hard to reject attributes that are higher than maxtype - we already pass that to nla_parse() wherever we call it, but we'd have to find a way to make it optional I guess, for compatibility reasons. Perhaps with a warning, like attribute validation. For genetlink, a flag in the family (something like "strict attribute validation") would be easy, but for "netlink proper" we have a lot of nlmsg_parse() calls to patch, and/or replace by nlmsg_parse_strict(). I guess we should 1) implement nlmsg_parse_strict() for those new things that want it strictly - greenfield type stuff that doesn't need to work with existing applications 2) add a warning to nlmsg_parse() when a too high attribute is encountered 3) eventually replace nlmsg_parse() calls by nlmsg_parse_strict() and see what breaks? :-) We won't be able to rely on that any time soon though (unless userspace first checks with a guaranteed rejected attribute, e.g. one that has NLA_REJECT, perhaps the u64 pad attributes could be marked such since the kernel can't assume alignment anyway) Perhaps we also have too many calls to nlmsg_parse() without a policy, but that's orthogonal to this check. On a second level though, with complex things like nl80211 it's often not clear at all which attributes are used with which commands. Some attributes (like NL80211_ATTR_IFINDEX) are (almost) universal, but there are others that aren't. Perhaps this isn't all that important, since if you try to trigger scanning and at the same time tell the kernel about a key index, that clearly makes no sense at all. OTOH, we have no good way of discovering what attribute is used where - we (try to) document this well in the nl80211.h kernel-doc, but that isn't always complete. So more introspection (of sorts) could be useful. While we're talking about wishlist, I'm also toying with the idea of having some sort of generic mechanism to convert netlink attributes to/from structs, for internal kernel representation; so far though I haven't been able to come up with anything useful. johannes
