Linux kernel mirror (for testing)
git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel
os
linux
Merge branch 'genetlink-apply-reject-policy-for-split-ops-on-the-dispatch-path'
Jakub Kicinski says:
====================
genetlink: apply reject policy for split ops on the dispatch path
Looks like I somehow missed adding default reject policies to commands
in families using split Netlink ops. I realized this randomly trying
to dump page pools for a specific device and always getting all of them
back. The per-device dump is simply not implemented so the request
should have been rejected. Patch 2 is the real change, the rest is
just accompaniment.
====================
Link: https://patch.msgid.link/20260311032839.417748-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
+186
-21
+10
-10
net/netlink/genetlink.c
+10
-10
net/netlink/genetlink.c
···
92
92
static unsigned long *mc_groups = &mc_group_start;
93
93
static unsigned long mc_groups_longs = 1;
94
94
95
-
/* We need the last attribute with non-zero ID therefore a 2-entry array */
96
95
static struct nla_policy genl_policy_reject_all[] = {
97
-
{ .type = NLA_REJECT },
98
96
{ .type = NLA_REJECT },
99
97
};
100
98
···
104
106
genl_op_fill_in_reject_policy(const struct genl_family *family,
105
107
struct genl_ops *op)
106
108
{
107
-
BUILD_BUG_ON(ARRAY_SIZE(genl_policy_reject_all) - 1 != 1);
108
-
109
109
if (op->policy || op->cmd < family->resv_start_op)
110
110
return;
111
111
112
112
op->policy = genl_policy_reject_all;
113
-
op->maxattr = 1;
114
113
}
115
114
116
115
static void
···
118
123
return;
119
124
120
125
op->policy = genl_policy_reject_all;
121
-
op->maxattr = 1;
122
126
}
123
127
124
128
static const struct genl_family *genl_family_find_byid(unsigned int id)
···
244
250
if (family->split_ops[i].cmd == cmd &&
245
251
family->split_ops[i].flags & flag) {
246
252
*op = family->split_ops[i];
253
+
genl_op_fill_in_reject_policy_split(family, op);
247
254
return 0;
248
255
}
249
256
···
929
934
struct nlattr **attrbuf;
930
935
int err;
931
936
932
-
if (!ops->maxattr)
937
+
if (!ops->policy)
933
938
return NULL;
934
939
935
-
attrbuf = kmalloc_objs(struct nlattr *, ops->maxattr + 1);
936
-
if (!attrbuf)
937
-
return ERR_PTR(-ENOMEM);
940
+
if (ops->maxattr) {
941
+
attrbuf = kmalloc_objs(struct nlattr *, ops->maxattr + 1);
942
+
if (!attrbuf)
943
+
return ERR_PTR(-ENOMEM);
944
+
} else {
945
+
/* Reject all policy, __nlmsg_parse() will just validate */
946
+
attrbuf = NULL;
947
+
}
938
948
939
949
err = __nlmsg_parse(nlh, hdrlen, attrbuf, ops->maxattr, ops->policy,
940
950
validate, extack);
+2
-2
net/netlink/policy.c
+2
-2
net/netlink/policy.c
···
31
31
struct netlink_policy_dump_state *state = *statep;
32
32
unsigned int old_n_alloc, n_alloc, i;
33
33
34
-
if (!policy || !maxtype)
34
+
if (!policy)
35
35
return 0;
36
36
37
37
for (i = 0; i < state->n_alloc; i++) {
···
85
85
{
86
86
unsigned int i;
87
87
88
-
if (WARN_ON(!policy || !maxtype))
88
+
if (WARN_ON(!policy))
89
89
return 0;
90
90
91
91
for (i = 0; i < state->n_alloc; i++) {
+1
tools/testing/selftests/net/Makefile
+1
tools/testing/selftests/net/Makefile
+3
-2
tools/testing/selftests/net/lib/py/__init__.py
+3
-2
tools/testing/selftests/net/lib/py/__init__.py
···
15
15
from .utils import CmdExitFailure, fd_read_timeout, cmd, bkg, defer, \
16
16
bpftool, ip, ethtool, bpftrace, rand_port, rand_ports, wait_port_listen, \
17
17
wait_file, tool
18
-
from .ynl import NlError, YnlFamily, EthtoolFamily, NetdevFamily, RtnlFamily, RtnlAddrFamily
18
+
from .ynl import NlError, NlctrlFamily, YnlFamily, \
19
+
EthtoolFamily, NetdevFamily, RtnlFamily, RtnlAddrFamily
19
20
from .ynl import NetshaperFamily, DevlinkFamily, PSPFamily, Netlink
20
21
21
22
__all__ = ["KSRC",
···
32
31
"NetdevSim", "NetdevSimDev",
33
32
"NetshaperFamily", "DevlinkFamily", "PSPFamily", "NlError",
34
33
"YnlFamily", "EthtoolFamily", "NetdevFamily", "RtnlFamily",
35
-
"RtnlAddrFamily", "Netlink"]
34
+
"NlctrlFamily", "RtnlAddrFamily", "Netlink"]
+9
-1
tools/testing/selftests/net/lib/py/ynl.py
+9
-1
tools/testing/selftests/net/lib/py/ynl.py
···
30
30
__all__ = [
31
31
"NlError", "NlPolicy", "Netlink", "YnlFamily", "SPEC_PATH",
32
32
"EthtoolFamily", "RtnlFamily", "RtnlAddrFamily",
33
-
"NetdevFamily", "NetshaperFamily", "DevlinkFamily", "PSPFamily",
33
+
"NetdevFamily", "NetshaperFamily", "NlctrlFamily", "DevlinkFamily",
34
+
"PSPFamily",
34
35
]
35
36
36
37
#
···
63
62
def __init__(self, recv_size=0):
64
63
super().__init__((SPEC_PATH / Path('net_shaper.yaml')).as_posix(),
65
64
schema='', recv_size=recv_size)
65
+
66
+
67
+
class NlctrlFamily(YnlFamily):
68
+
def __init__(self, recv_size=0):
69
+
super().__init__((SPEC_PATH / Path('nlctrl.yaml')).as_posix(),
70
+
schema='', recv_size=recv_size)
71
+
66
72
67
73
class DevlinkFamily(YnlFamily):
68
74
def __init__(self, recv_size=0):
+26
-6
tools/testing/selftests/net/nl_netdev.py
+26
-6
tools/testing/selftests/net/nl_netdev.py
···
1
1
#!/usr/bin/env python3
2
2
# SPDX-License-Identifier: GPL-2.0
3
3
4
-
import time
4
+
"""
5
+
Tests for the netdev netlink family.
6
+
"""
7
+
8
+
import errno
5
9
from os import system
6
-
from lib.py import ksft_run, ksft_exit, ksft_pr
7
-
from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_busy_wait
8
-
from lib.py import NetdevFamily, NetdevSimDev, ip
10
+
from lib.py import ksft_run, ksft_exit
11
+
from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_raises, ksft_busy_wait
12
+
from lib.py import NetdevFamily, NetdevSimDev, NlError, ip
9
13
10
14
11
15
def empty_check(nf) -> None:
···
21
17
lo_info = nf.dev_get({"ifindex": 1})
22
18
ksft_eq(len(lo_info['xdp-features']), 0)
23
19
ksft_eq(len(lo_info['xdp-rx-metadata-features']), 0)
20
+
21
+
22
+
def dev_dump_reject_attr(nf) -> None:
23
+
"""Test that dev-get dump rejects attributes (no dump request policy)."""
24
+
with ksft_raises(NlError) as cm:
25
+
nf.dev_get({'ifindex': 1}, dump=True)
26
+
ksft_eq(cm.exception.nl_msg.error, -errno.EINVAL)
27
+
ksft_eq(cm.exception.nl_msg.extack['msg'], 'Unknown attribute type')
28
+
ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.ifindex')
24
29
25
30
26
31
def napi_list_check(nf) -> None:
···
256
243
257
244
258
245
def main() -> None:
246
+
""" Ksft boiler plate main """
259
247
nf = NetdevFamily()
260
-
ksft_run([empty_check, lo_check, page_pool_check, napi_list_check,
261
-
dev_set_threaded, napi_set_threaded, nsim_rxq_reset_down],
248
+
ksft_run([empty_check,
249
+
lo_check,
250
+
dev_dump_reject_attr,
251
+
napi_list_check,
252
+
napi_set_threaded,
253
+
dev_set_threaded,
254
+
nsim_rxq_reset_down,
255
+
page_pool_check],
262
256
args=(nf, ))
263
257
ksft_exit()
264
258
+135
tools/testing/selftests/net/nl_nlctrl.py
+135
tools/testing/selftests/net/nl_nlctrl.py
···
1
+
#!/usr/bin/env python3
2
+
# SPDX-License-Identifier: GPL-2.0
3
+
4
+
"""
5
+
Tests for the nlctrl genetlink family (family info and policy dumps).
6
+
"""
7
+
8
+
from lib.py import ksft_run, ksft_exit
9
+
from lib.py import ksft_eq, ksft_ge, ksft_true, ksft_in, ksft_not_in
10
+
from lib.py import NetdevFamily, EthtoolFamily, NlctrlFamily
11
+
12
+
13
+
def getfamily_do(ctrl) -> None:
14
+
"""Query a single family by name and validate its ops."""
15
+
fam = ctrl.getfamily({'family-name': 'netdev'})
16
+
ksft_eq(fam['family-name'], 'netdev')
17
+
ksft_true(fam['family-id'] > 0)
18
+
19
+
# The format of ops is quite odd, [{$idx: {"id"...}}, {$idx: {"id"...}}]
20
+
# Discard the indices and re-key by command id.
21
+
ops_by_id = {v['id']: v for op in fam['ops'] for v in op.values()}
22
+
ksft_eq(len(ops_by_id), len(fam['ops']))
23
+
24
+
# All ops should have a policy (either do or dump has one)
25
+
for op in ops_by_id.values():
26
+
ksft_in('cmd-cap-haspol', op['flags'],
27
+
comment=f"op {op['id']} missing haspol")
28
+
29
+
# dev-get (id 1) should support both do and dump
30
+
ksft_in('cmd-cap-do', ops_by_id[1]['flags'])
31
+
ksft_in('cmd-cap-dump', ops_by_id[1]['flags'])
32
+
33
+
# qstats-get (id 12) is dump-only
34
+
ksft_not_in('cmd-cap-do', ops_by_id[12]['flags'])
35
+
ksft_in('cmd-cap-dump', ops_by_id[12]['flags'])
36
+
37
+
# napi-set (id 14) is do-only and requires admin
38
+
ksft_in('cmd-cap-do', ops_by_id[14]['flags'])
39
+
ksft_not_in('cmd-cap-dump', ops_by_id[14]['flags'])
40
+
ksft_in('admin-perm', ops_by_id[14]['flags'])
41
+
42
+
# Notification-only commands (dev-add/del/change-ntf etc.) must
43
+
# not appear in the ops list since they have no do/dump handlers.
44
+
for ntf_id in [2, 3, 4, 6, 7, 8]:
45
+
ksft_not_in(ntf_id, ops_by_id,
46
+
comment=f"ntf-only cmd {ntf_id} should not be in ops")
47
+
48
+
49
+
def getfamily_dump(ctrl) -> None:
50
+
"""Dump all families and verify expected entries."""
51
+
families = ctrl.getfamily({}, dump=True)
52
+
ksft_ge(len(families), 2)
53
+
54
+
names = [f['family-name'] for f in families]
55
+
ksft_in('nlctrl', names, comment="nlctrl not found in family dump")
56
+
ksft_in('netdev', names, comment="netdev not found in family dump")
57
+
58
+
59
+
def getpolicy_dump(_ctrl) -> None:
60
+
"""Dump policies for ops using get_policy() and validate results.
61
+
62
+
Test with netdev (split ops) where do and dump can have different
63
+
policies, and with ethtool (full ops) where they always share one.
64
+
"""
65
+
# -- netdev (split ops) --
66
+
ndev = NetdevFamily()
67
+
68
+
# dev-get: do has a real policy with ifindex, dump has no policy
69
+
# (only the reject-all policy with maxattr=0)
70
+
pol = ndev.get_policy('dev-get', 'do')
71
+
ksft_in('ifindex', pol.attrs,
72
+
comment="dev-get do policy should have ifindex")
73
+
ksft_eq(pol.attrs.get('ifindex', {}).get('type'), 'u32')
74
+
75
+
pol_dump = ndev.get_policy('dev-get', 'dump')
76
+
ksft_eq(len(pol_dump.attrs), 0,
77
+
comment="dev-get should not accept any attrs")
78
+
79
+
# napi-get: both do and dump have real policies
80
+
pol_do = ndev.get_policy('napi-get', 'do')
81
+
ksft_ge(len(pol_do.attrs), 1)
82
+
83
+
pol_dump = ndev.get_policy('napi-get', 'dump')
84
+
ksft_ge(len(pol_dump.attrs), 1)
85
+
86
+
# -- ethtool (full ops) --
87
+
et = EthtoolFamily()
88
+
89
+
# strset-get (has both do and dump, full ops share policy)
90
+
pol_do = et.get_policy('strset-get', 'do')
91
+
ksft_ge(len(pol_do.attrs), 1, comment="strset-get should have a do policy")
92
+
93
+
pol_dump = et.get_policy('strset-get', 'dump')
94
+
ksft_ge(len(pol_dump.attrs), 1,
95
+
comment="strset-get should have a dump policy")
96
+
97
+
# Same policy means same attribute names
98
+
ksft_eq(set(pol_do.attrs.keys()), set(pol_dump.attrs.keys()))
99
+
100
+
# linkinfo-set is do-only (SET command), no dump
101
+
pol_do = et.get_policy('linkinfo-set', 'do')
102
+
ksft_ge(len(pol_do.attrs), 1,
103
+
comment="linkinfo-set should have a do policy")
104
+
105
+
pol_dump = et.get_policy('linkinfo-set', 'dump')
106
+
ksft_eq(pol_dump, None,
107
+
comment="linkinfo-set should not have a dump policy")
108
+
109
+
110
+
def getpolicy_by_op(_ctrl) -> None:
111
+
"""Query policy for specific ops, check attr names are resolved."""
112
+
ndev = NetdevFamily()
113
+
114
+
# dev-get do policy should have named attributes from the spec
115
+
pol = ndev.get_policy('dev-get', 'do')
116
+
ksft_ge(len(pol.attrs), 1)
117
+
# All attr names should be resolved (no 'attr-N' fallbacks)
118
+
for name in pol.attrs:
119
+
ksft_true(not name.startswith('attr-'),
120
+
comment=f"unresolved attr name: {name}")
121
+
122
+
123
+
def main() -> None:
124
+
""" Ksft boiler plate main """
125
+
ctrl = NlctrlFamily()
126
+
ksft_run([getfamily_do,
127
+
getfamily_dump,
128
+
getpolicy_dump,
129
+
getpolicy_by_op],
130
+
args=(ctrl, ))
131
+
ksft_exit()
132
+
133
+
134
+
if __name__ == "__main__":
135
+
main()