Commit 4fa86555d1cd ("genetlink: piggy back on resv_op to default to
a reject policy") added genl_policy_reject_all to ensure that ops
without an explicit policy reject all attributes rather than silently
accepting them.The reject policy had maxattr of 1. Passing info->attrs of size 2 may surprise families. Devlink, for instance, assumes that if info->attrs is set it's safe to access DEVLINK_ATTR_BUS_NAME (1) and DEVLINK_ATTR_DEV_NAME (2). Before plugging reject policies into split ops we need to make sure the genetlink code will not populate info->attrs if family had no explicit policy for the op. While even shared code paths within the families can figure out that given op has no policy fairly easily themselves, passing attrs with fixed size of 2 feels fairly useless and error prone. This change has no user-visible impact, reject attrs are not reported to the user space via getpolicy. We do have to remove the safety check in netlink_policy_dump_get_policy_idx() but it seems to have been there to catch likely faulty input, the code can handle maxattr = 0 just fine. Signed-off-by: Jakub Kicinski <[email protected]> --- net/netlink/genetlink.c | 19 +++++++++---------- net/netlink/policy.c | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index a23d4c51c089..c00f0586c8d6 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -92,10 +92,8 @@ static unsigned long mc_group_start = 0x3 | BIT(GENL_ID_CTRL) | static unsigned long *mc_groups = &mc_group_start; static unsigned long mc_groups_longs = 1; -/* We need the last attribute with non-zero ID therefore a 2-entry array */ static struct nla_policy genl_policy_reject_all[] = { { .type = NLA_REJECT }, - { .type = NLA_REJECT }, }; static int genl_ctrl_event(int event, const struct genl_family *family, @@ -106,13 +104,10 @@ static void genl_op_fill_in_reject_policy(const struct genl_family *family, struct genl_ops *op) { - BUILD_BUG_ON(ARRAY_SIZE(genl_policy_reject_all) - 1 != 1); - if (op->policy || op->cmd < family->resv_start_op) return; op->policy = genl_policy_reject_all; - op->maxattr = 1; } static void @@ -123,7 +118,6 @@ genl_op_fill_in_reject_policy_split(const struct genl_family *family, return; op->policy = genl_policy_reject_all; - op->maxattr = 1; } static const struct genl_family *genl_family_find_byid(unsigned int id) @@ -934,12 +928,17 @@ genl_family_rcv_msg_attrs_parse(const struct genl_family *family, struct nlattr **attrbuf; int err; - if (!ops->maxattr) + if (!ops->policy) return NULL; - attrbuf = kmalloc_objs(struct nlattr *, ops->maxattr + 1); - if (!attrbuf) - return ERR_PTR(-ENOMEM); + if (ops->maxattr) { + attrbuf = kmalloc_objs(struct nlattr *, ops->maxattr + 1); + if (!attrbuf) + return ERR_PTR(-ENOMEM); + } else { + /* Reject all policy, __nlmsg_parse() will just validate */ + attrbuf = NULL; + } err = __nlmsg_parse(nlh, hdrlen, attrbuf, ops->maxattr, ops->policy, validate, extack); diff --git a/net/netlink/policy.c b/net/netlink/policy.c index f39cd7cc4fb5..08b006c48f06 100644 --- a/net/netlink/policy.c +++ b/net/netlink/policy.c @@ -31,7 +31,7 @@ static int add_policy(struct netlink_policy_dump_state **statep, struct netlink_policy_dump_state *state = *statep; unsigned int old_n_alloc, n_alloc, i; - if (!policy || !maxtype) + if (!policy) return 0; for (i = 0; i < state->n_alloc; i++) { @@ -85,7 +85,7 @@ int netlink_policy_dump_get_policy_idx(struct netlink_policy_dump_state *state, { unsigned int i; - if (WARN_ON(!policy || !maxtype)) + if (WARN_ON(!policy)) return 0; for (i = 0; i < state->n_alloc; i++) { -- 2.53.0
