On Thu, 2020-10-01 at 11:30 -0700, Jakub Kicinski wrote:
>
> +++ b/net/netlink/genetlink.c
> @@ -1119,6 +1119,7 @@ static const struct nla_policy ctrl_policy_policy[] = {
> [CTRL_ATTR_FAMILY_ID] = { .type = NLA_U16 },
> [CTRL_ATTR_FAMILY_NAME] = { .type = NLA_NUL_STRING,
> .len = GENL_NAMSIZ - 1 },
> + [CTRL_ATTR_OP] = { .type = NLA_U8 },
I slightly wonder if we shouldn't make this wider. There's no real
*benefit* to using a u8 since, due to padding, the attribute actually
has the same size anyway (up to U32), and we also still need to validate
it actually exists.
However, if we ever run out of command numbers in some family (nl80211
is almost half way :-) ) then I believe we still have some reserved
space in the genl header that we could use for extensions, but if then
we have to also go around and change a bunch of other interfaces like
this, it'll just be so much harder, and ... we'd probably just be
tempted to multiplex onto an "extension command" or something? Or maybe
then we should have a separate genl family at that point?
(Internal interfaces are much easier to change)
Dunno. Just a thought. We probably won't ever get there, it just sort of
rubs me the wrong way that we'd be restricting this in an "unfixable"
API for no real reason :)
> - if (!rt->policy)
> + if (tb[CTRL_ATTR_OP]) {
> + err = genl_get_cmd(nla_get_u8(tb[CTRL_ATTR_OP]), rt, &op);
OK, so maybe if we make that wider we also have to make the argument to
genl_get_cmd() wider so we don't erroneously return op 1 if you ask for
257, but that's in an at least 32-bit register anyway, presumably?
johannes
