genetlink: use maxattr of 0 for the reject policy

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.

Link: https://patch.msgid.link/20260311032839.417748-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index a23d4c51c089d2c8dcae02c0dbe0ae05358092a7..c00f0586c8d6550b4b0a61c22de7da3d5b078848 100644 (file)
--- 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 f39cd7cc4fb5ac7cb758a418d0e660720eec9e61..08b006c48f06e3776c490bb5e726f7f6fb93d912 100644 (file)
--- 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++) {