* [PATCH net-next v2 1/4] genetlink: use maxattr of 0 for the reject policy
2026-03-11 3:28 [PATCH net-next v2 0/4] genetlink: apply reject policy for split ops on the dispatch path Jakub Kicinski
@ 2026-03-11 3:28 ` Jakub Kicinski
2026-03-11 3:28 ` [PATCH net-next v2 2/4] genetlink: apply reject policy for split ops on the dispatch path Jakub Kicinski
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-03-11 3:28 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, shuah,
linux-kselftest, Jakub Kicinski
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 <kuba@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next v2 3/4] selftests: net: make sure that Netlink rejects unknown attrs in dump
2026-03-11 3:28 [PATCH net-next v2 0/4] genetlink: apply reject policy for split ops on the dispatch path Jakub Kicinski
2026-03-11 3:28 ` [PATCH net-next v2 1/4] genetlink: use maxattr of 0 for the reject policy Jakub Kicinski
2026-03-11 3:28 ` [PATCH net-next v2 2/4] genetlink: apply reject policy for split ops on the dispatch path Jakub Kicinski
@ 2026-03-11 3:28 ` Jakub Kicinski
2026-03-11 3:28 ` [PATCH net-next v2 4/4] selftests: net: add test for Netlink policy dumps Jakub Kicinski
2026-03-13 1:20 ` [PATCH net-next v2 0/4] genetlink: apply reject policy for split ops on the dispatch path patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-03-11 3:28 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, shuah,
linux-kselftest, Jakub Kicinski
Add a test case for rejecting attrs if policy is not set.
dev_get dump has no input policy (accepts no attrs).
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: shuah@kernel.org
CC: linux-kselftest@vger.kernel.org
---
tools/testing/selftests/net/nl_netdev.py | 32 +++++++++++++++++++-----
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/net/nl_netdev.py b/tools/testing/selftests/net/nl_netdev.py
index 5c66421ab8aa..eff55c64a012 100755
--- a/tools/testing/selftests/net/nl_netdev.py
+++ b/tools/testing/selftests/net/nl_netdev.py
@@ -1,11 +1,15 @@
#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0
-import time
+"""
+Tests for the netdev netlink family.
+"""
+
+import errno
from os import system
-from lib.py import ksft_run, ksft_exit, ksft_pr
-from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_busy_wait
-from lib.py import NetdevFamily, NetdevSimDev, ip
+from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_raises, ksft_busy_wait
+from lib.py import NetdevFamily, NetdevSimDev, NlError, ip
def empty_check(nf) -> None:
@@ -19,6 +23,15 @@ from lib.py import NetdevFamily, NetdevSimDev, ip
ksft_eq(len(lo_info['xdp-rx-metadata-features']), 0)
+def dev_dump_reject_attr(nf) -> None:
+ """Test that dev-get dump rejects attributes (no dump request policy)."""
+ with ksft_raises(NlError) as cm:
+ nf.dev_get({'ifindex': 1}, dump=True)
+ ksft_eq(cm.exception.nl_msg.error, -errno.EINVAL)
+ ksft_eq(cm.exception.nl_msg.extack['msg'], 'Unknown attribute type')
+ ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.ifindex')
+
+
def napi_list_check(nf) -> None:
with NetdevSimDev(queue_count=100) as nsimdev:
nsim = nsimdev.nsims[0]
@@ -243,9 +256,16 @@ from lib.py import NetdevFamily, NetdevSimDev, ip
def main() -> None:
+ """ Ksft boiler plate main """
nf = NetdevFamily()
- ksft_run([empty_check, lo_check, page_pool_check, napi_list_check,
- dev_set_threaded, napi_set_threaded, nsim_rxq_reset_down],
+ ksft_run([empty_check,
+ lo_check,
+ dev_dump_reject_attr,
+ napi_list_check,
+ napi_set_threaded,
+ dev_set_threaded,
+ nsim_rxq_reset_down,
+ page_pool_check],
args=(nf, ))
ksft_exit()
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next v2 4/4] selftests: net: add test for Netlink policy dumps
2026-03-11 3:28 [PATCH net-next v2 0/4] genetlink: apply reject policy for split ops on the dispatch path Jakub Kicinski
` (2 preceding siblings ...)
2026-03-11 3:28 ` [PATCH net-next v2 3/4] selftests: net: make sure that Netlink rejects unknown attrs in dump Jakub Kicinski
@ 2026-03-11 3:28 ` Jakub Kicinski
2026-03-13 1:20 ` [PATCH net-next v2 0/4] genetlink: apply reject policy for split ops on the dispatch path patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-03-11 3:28 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, shuah,
linux-kselftest, Jakub Kicinski
Add validation for the nlctrl family, accessing family info and
dumping policies.
TAP version 13
1..4
ok 1 nl_nlctrl.getfamily_do
ok 2 nl_nlctrl.getfamily_dump
ok 3 nl_nlctrl.getpolicy_dump
ok 4 nl_nlctrl.getpolicy_by_op
# Totals: pass:4 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
tools/testing/selftests/net/Makefile | 1 +
.../testing/selftests/net/lib/py/__init__.py | 5 +-
tools/testing/selftests/net/lib/py/ynl.py | 10 +-
tools/testing/selftests/net/nl_nlctrl.py | 135 ++++++++++++++++++
4 files changed, 148 insertions(+), 3 deletions(-)
create mode 100755 tools/testing/selftests/net/nl_nlctrl.py
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index a24ea64e2ae8..6bced3ed798b 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -65,6 +65,7 @@ TEST_PROGS := \
netns-name.sh \
netns-sysctl.sh \
nl_netdev.py \
+ nl_nlctrl.py \
pmtu.sh \
psock_snd.sh \
reuseaddr_ports_exhausted.sh \
diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
index 54e84282781c..e0c920041a58 100644
--- a/tools/testing/selftests/net/lib/py/__init__.py
+++ b/tools/testing/selftests/net/lib/py/__init__.py
@@ -15,7 +15,8 @@ from .nsim import NetdevSim, NetdevSimDev
from .utils import CmdExitFailure, fd_read_timeout, cmd, bkg, defer, \
bpftool, ip, ethtool, bpftrace, rand_port, rand_ports, wait_port_listen, \
wait_file, tool
-from .ynl import NlError, YnlFamily, EthtoolFamily, NetdevFamily, RtnlFamily, RtnlAddrFamily
+from .ynl import NlError, NlctrlFamily, YnlFamily, \
+ EthtoolFamily, NetdevFamily, RtnlFamily, RtnlAddrFamily
from .ynl import NetshaperFamily, DevlinkFamily, PSPFamily, Netlink
__all__ = ["KSRC",
@@ -31,4 +32,4 @@ __all__ = ["KSRC",
"NetdevSim", "NetdevSimDev",
"NetshaperFamily", "DevlinkFamily", "PSPFamily", "NlError",
"YnlFamily", "EthtoolFamily", "NetdevFamily", "RtnlFamily",
- "RtnlAddrFamily", "Netlink"]
+ "NlctrlFamily", "RtnlAddrFamily", "Netlink"]
diff --git a/tools/testing/selftests/net/lib/py/ynl.py b/tools/testing/selftests/net/lib/py/ynl.py
index e8aa97936f6c..2e567062aa6c 100644
--- a/tools/testing/selftests/net/lib/py/ynl.py
+++ b/tools/testing/selftests/net/lib/py/ynl.py
@@ -30,7 +30,8 @@ from .ksft import ksft_pr, ktap_result
__all__ = [
"NlError", "NlPolicy", "Netlink", "YnlFamily", "SPEC_PATH",
"EthtoolFamily", "RtnlFamily", "RtnlAddrFamily",
- "NetdevFamily", "NetshaperFamily", "DevlinkFamily", "PSPFamily",
+ "NetdevFamily", "NetshaperFamily", "NlctrlFamily", "DevlinkFamily",
+ "PSPFamily",
]
#
@@ -63,6 +64,13 @@ __all__ = [
super().__init__((SPEC_PATH / Path('net_shaper.yaml')).as_posix(),
schema='', recv_size=recv_size)
+
+class NlctrlFamily(YnlFamily):
+ def __init__(self, recv_size=0):
+ super().__init__((SPEC_PATH / Path('nlctrl.yaml')).as_posix(),
+ schema='', recv_size=recv_size)
+
+
class DevlinkFamily(YnlFamily):
def __init__(self, recv_size=0):
super().__init__((SPEC_PATH / Path('devlink.yaml')).as_posix(),
diff --git a/tools/testing/selftests/net/nl_nlctrl.py b/tools/testing/selftests/net/nl_nlctrl.py
new file mode 100755
index 000000000000..d19e206c2c02
--- /dev/null
+++ b/tools/testing/selftests/net/nl_nlctrl.py
@@ -0,0 +1,135 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+Tests for the nlctrl genetlink family (family info and policy dumps).
+"""
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_eq, ksft_ge, ksft_true, ksft_in, ksft_not_in
+from lib.py import NetdevFamily, EthtoolFamily, NlctrlFamily
+
+
+def getfamily_do(ctrl) -> None:
+ """Query a single family by name and validate its ops."""
+ fam = ctrl.getfamily({'family-name': 'netdev'})
+ ksft_eq(fam['family-name'], 'netdev')
+ ksft_true(fam['family-id'] > 0)
+
+ # The format of ops is quite odd, [{$idx: {"id"...}}, {$idx: {"id"...}}]
+ # Discard the indices and re-key by command id.
+ ops_by_id = {v['id']: v for op in fam['ops'] for v in op.values()}
+ ksft_eq(len(ops_by_id), len(fam['ops']))
+
+ # All ops should have a policy (either do or dump has one)
+ for op in ops_by_id.values():
+ ksft_in('cmd-cap-haspol', op['flags'],
+ comment=f"op {op['id']} missing haspol")
+
+ # dev-get (id 1) should support both do and dump
+ ksft_in('cmd-cap-do', ops_by_id[1]['flags'])
+ ksft_in('cmd-cap-dump', ops_by_id[1]['flags'])
+
+ # qstats-get (id 12) is dump-only
+ ksft_not_in('cmd-cap-do', ops_by_id[12]['flags'])
+ ksft_in('cmd-cap-dump', ops_by_id[12]['flags'])
+
+ # napi-set (id 14) is do-only and requires admin
+ ksft_in('cmd-cap-do', ops_by_id[14]['flags'])
+ ksft_not_in('cmd-cap-dump', ops_by_id[14]['flags'])
+ ksft_in('admin-perm', ops_by_id[14]['flags'])
+
+ # Notification-only commands (dev-add/del/change-ntf etc.) must
+ # not appear in the ops list since they have no do/dump handlers.
+ for ntf_id in [2, 3, 4, 6, 7, 8]:
+ ksft_not_in(ntf_id, ops_by_id,
+ comment=f"ntf-only cmd {ntf_id} should not be in ops")
+
+
+def getfamily_dump(ctrl) -> None:
+ """Dump all families and verify expected entries."""
+ families = ctrl.getfamily({}, dump=True)
+ ksft_ge(len(families), 2)
+
+ names = [f['family-name'] for f in families]
+ ksft_in('nlctrl', names, comment="nlctrl not found in family dump")
+ ksft_in('netdev', names, comment="netdev not found in family dump")
+
+
+def getpolicy_dump(_ctrl) -> None:
+ """Dump policies for ops using get_policy() and validate results.
+
+ Test with netdev (split ops) where do and dump can have different
+ policies, and with ethtool (full ops) where they always share one.
+ """
+ # -- netdev (split ops) --
+ ndev = NetdevFamily()
+
+ # dev-get: do has a real policy with ifindex, dump has no policy
+ # (only the reject-all policy with maxattr=0)
+ pol = ndev.get_policy('dev-get', 'do')
+ ksft_in('ifindex', pol.attrs,
+ comment="dev-get do policy should have ifindex")
+ ksft_eq(pol.attrs.get('ifindex', {}).get('type'), 'u32')
+
+ pol_dump = ndev.get_policy('dev-get', 'dump')
+ ksft_eq(len(pol_dump.attrs), 0,
+ comment="dev-get should not accept any attrs")
+
+ # napi-get: both do and dump have real policies
+ pol_do = ndev.get_policy('napi-get', 'do')
+ ksft_ge(len(pol_do.attrs), 1)
+
+ pol_dump = ndev.get_policy('napi-get', 'dump')
+ ksft_ge(len(pol_dump.attrs), 1)
+
+ # -- ethtool (full ops) --
+ et = EthtoolFamily()
+
+ # strset-get (has both do and dump, full ops share policy)
+ pol_do = et.get_policy('strset-get', 'do')
+ ksft_ge(len(pol_do.attrs), 1, comment="strset-get should have a do policy")
+
+ pol_dump = et.get_policy('strset-get', 'dump')
+ ksft_ge(len(pol_dump.attrs), 1,
+ comment="strset-get should have a dump policy")
+
+ # Same policy means same attribute names
+ ksft_eq(set(pol_do.attrs.keys()), set(pol_dump.attrs.keys()))
+
+ # linkinfo-set is do-only (SET command), no dump
+ pol_do = et.get_policy('linkinfo-set', 'do')
+ ksft_ge(len(pol_do.attrs), 1,
+ comment="linkinfo-set should have a do policy")
+
+ pol_dump = et.get_policy('linkinfo-set', 'dump')
+ ksft_eq(pol_dump, None,
+ comment="linkinfo-set should not have a dump policy")
+
+
+def getpolicy_by_op(_ctrl) -> None:
+ """Query policy for specific ops, check attr names are resolved."""
+ ndev = NetdevFamily()
+
+ # dev-get do policy should have named attributes from the spec
+ pol = ndev.get_policy('dev-get', 'do')
+ ksft_ge(len(pol.attrs), 1)
+ # All attr names should be resolved (no 'attr-N' fallbacks)
+ for name in pol.attrs:
+ ksft_true(not name.startswith('attr-'),
+ comment=f"unresolved attr name: {name}")
+
+
+def main() -> None:
+ """ Ksft boiler plate main """
+ ctrl = NlctrlFamily()
+ ksft_run([getfamily_do,
+ getfamily_dump,
+ getpolicy_dump,
+ getpolicy_by_op],
+ args=(ctrl, ))
+ ksft_exit()
+
+
+if __name__ == "__main__":
+ main()
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread