summaryrefslogtreecommitdiffstats
path: root/net/netlink/genetlink.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netPaolo Abeni2024-10-211-14/+14
|\ | | | | | | | | | | | | | | | | | | | | Cross-merge networking fixes after downstream PR (net-6.12-rc4). Conflicts: 107a034d5c1e ("net/mlx5: qos: Store rate groups in a qos domain") 1da9cfd6c41c ("net/mlx5: Unregister notifier on eswitch init failure") Signed-off-by: Paolo Abeni <pabeni@redhat.com>
| * genetlink: hold RCU in genlmsg_mcast()Eric Dumazet2024-10-161-14/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While running net selftests with CONFIG_PROVE_RCU_LIST=y I saw one lockdep splat [1]. genlmsg_mcast() uses for_each_net_rcu(), and must therefore hold RCU. Instead of letting all callers guard genlmsg_multicast_allns() with a rcu_read_lock()/rcu_read_unlock() pair, do it in genlmsg_mcast(). This also means the @flags parameter is useless, we need to always use GFP_ATOMIC. [1] [10882.424136] ============================= [10882.424166] WARNING: suspicious RCU usage [10882.424309] 6.12.0-rc2-virtme #1156 Not tainted [10882.424400] ----------------------------- [10882.424423] net/netlink/genetlink.c:1940 RCU-list traversed in non-reader section!! [10882.424469] other info that might help us debug this: [10882.424500] rcu_scheduler_active = 2, debug_locks = 1 [10882.424744] 2 locks held by ip/15677: [10882.424791] #0: ffffffffb6b491b0 (cb_lock){++++}-{3:3}, at: genl_rcv (net/netlink/genetlink.c:1219) [10882.426334] #1: ffffffffb6b49248 (genl_mutex){+.+.}-{3:3}, at: genl_rcv_msg (net/netlink/genetlink.c:61 net/netlink/genetlink.c:57 net/netlink/genetlink.c:1209) [10882.426465] stack backtrace: [10882.426805] CPU: 14 UID: 0 PID: 15677 Comm: ip Not tainted 6.12.0-rc2-virtme #1156 [10882.426919] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [10882.427046] Call Trace: [10882.427131] <TASK> [10882.427244] dump_stack_lvl (lib/dump_stack.c:123) [10882.427335] lockdep_rcu_suspicious (kernel/locking/lockdep.c:6822) [10882.427387] genlmsg_multicast_allns (net/netlink/genetlink.c:1940 (discriminator 7) net/netlink/genetlink.c:1977 (discriminator 7)) [10882.427436] l2tp_tunnel_notify.constprop.0 (net/l2tp/l2tp_netlink.c:119) l2tp_netlink [10882.427683] l2tp_nl_cmd_tunnel_create (net/l2tp/l2tp_netlink.c:253) l2tp_netlink [10882.427748] genl_family_rcv_msg_doit (net/netlink/genetlink.c:1115) [10882.427834] genl_rcv_msg (net/netlink/genetlink.c:1195 net/netlink/genetlink.c:1210) [10882.427877] ? __pfx_l2tp_nl_cmd_tunnel_create (net/l2tp/l2tp_netlink.c:186) l2tp_netlink [10882.427927] ? __pfx_genl_rcv_msg (net/netlink/genetlink.c:1201) [10882.427959] netlink_rcv_skb (net/netlink/af_netlink.c:2551) [10882.428069] genl_rcv (net/netlink/genetlink.c:1220) [10882.428095] netlink_unicast (net/netlink/af_netlink.c:1332 net/netlink/af_netlink.c:1357) [10882.428140] netlink_sendmsg (net/netlink/af_netlink.c:1901) [10882.428210] ____sys_sendmsg (net/socket.c:729 (discriminator 1) net/socket.c:744 (discriminator 1) net/socket.c:2607 (discriminator 1)) Fixes: 33f72e6f0c67 ("l2tp : multicast notification to the registered listeners") Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: James Chapman <jchapman@katalix.com> Cc: Tom Parkin <tparkin@katalix.com> Cc: Johannes Berg <johannes.berg@intel.com> Link: https://patch.msgid.link/20241011171217.3166614-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* | genetlink: extend info user-storage to match NL cb ctxPaolo Abeni2024-10-101-2/+2
|/ | | | | | | | | | | | | | | | This allows a more uniform implementation of non-dump and dump operations, and will be used later in the series to avoid some per-operation allocation. Additionally rename the NL_ASSERT_DUMP_CTX_FITS macro, to fit a more extended usage. Suggested-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com> Link: https://patch.msgid.link/1130cc2896626b84587a2a5f96a5c6829638f4da.1728460186.git.pabeni@redhat.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* netlink: create a new header for internal genetlink symbolsJakub Kicinski2024-04-021-0/+2
| | | | | | | | | | | There are things in linux/genetlink.h which are only used under net/netlink/. Move them to a new local header. A new header with just 2 externs isn't great, but alternative would be to include af_netlink.h in genetlink.c which feels even worse. Link: https://lore.kernel.org/r/20240329175710.291749-2-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: fit NLMSG_DONE into same read() as familiesJakub Kicinski2024-03-061-5/+7
| | | | | | | | | | | | | | Make sure ctrl_fill_info() returns sensible error codes and propagate them out to netlink core. Let netlink core decide when to return skb->len and when to treat the exit as an error. Netlink core does better job at it, if we always return skb->len the core doesn't know when we're done dumping and NLMSG_DONE ends up in a separate read(). Reviewed-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: Add per family bind/unbind callbacksStanislaw Gruszka2024-02-161-0/+30
| | | | | | | | | | | | | | | | | Add genetlink family bind()/unbind() callbacks when adding/removing multicast group to/from netlink client socket via setsockopt() or bind() syscall. They can be used to track if consumers of netlink multicast messages emerge or disappear. Thus, a client implementing callbacks, can now send events only when there are active consumers, preventing unnecessary work when none exist. Suggested-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20240212161615.161935-2-stanislaw.gruszka@linux.intel.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: Use internal flags for multicast groupsIdo Schimmel2023-12-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | As explained in commit e03781879a0d ("drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group"), the "flags" field in the multicast group structure reuses uAPI flags despite the field not being exposed to user space. This makes it impossible to extend its use without adding new uAPI flags, which is inappropriate for internal kernel checks. Solve this by adding internal flags (i.e., "GENL_MCAST_*") and convert the existing users to use them instead of the uAPI flags. Tested using the reproducers in commit 44ec98ea5ea9 ("psample: Require 'CAP_NET_ADMIN' when joining "packets" group") and commit e03781879a0d ("drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group"). No functional changes intended. Signed-off-by: Ido Schimmel <idosch@nvidia.com> Reviewed-by: Mat Martineau <martineau@kernel.org> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: introduce per-sock family private storageJiri Pirko2023-12-191-1/+143
| | | | | | | | | | | | | | | Introduce an xarray for Generic netlink family to store per-socket private. Initialize this xarray only if family uses per-socket privs. Introduce genl_sk_priv_get() to get the socket priv pointer for a family and initialize it in case it does not exist. Introduce __genl_sk_priv_get() to obtain socket priv pointer for a family under RCU read lock. Allow family to specify the priv size, init() and destroy() callbacks. Signed-off-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
* drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" groupIdo Schimmel2023-12-071-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "NET_DM" generic netlink family notifies drop locations over the "events" multicast group. This is problematic since by default generic netlink allows non-root users to listen to these notifications. Fix by adding a new field to the generic netlink multicast group structure that when set prevents non-root users or root without the 'CAP_SYS_ADMIN' capability (in the user namespace owning the network namespace) from joining the group. Set this field for the "events" group. Use 'CAP_SYS_ADMIN' rather than 'CAP_NET_ADMIN' because of the nature of the information that is shared over this group. Note that the capability check in this case will always be performed against the initial user namespace since the family is not netns aware and only operates in the initial network namespace. A new field is added to the structure rather than using the "flags" field because the existing field uses uAPI flags and it is inappropriate to add a new uAPI flag for an internal kernel check. In net-next we can rework the "flags" field to use internal flags and fold the new field into it. But for now, in order to reduce the amount of changes, add a new field. Since the information can only be consumed by root, mark the control plane operations that start and stop the tracing as root-only using the 'GENL_ADMIN_PERM' flag. Tested using [1]. Before: # capsh -- -c ./dm_repo # capsh --drop=cap_sys_admin -- -c ./dm_repo After: # capsh -- -c ./dm_repo # capsh --drop=cap_sys_admin -- -c ./dm_repo Failed to join "events" multicast group [1] $ cat dm.c #include <stdio.h> #include <netlink/genl/ctrl.h> #include <netlink/genl/genl.h> #include <netlink/socket.h> int main(int argc, char **argv) { struct nl_sock *sk; int grp, err; sk = nl_socket_alloc(); if (!sk) { fprintf(stderr, "Failed to allocate socket\n"); return -1; } err = genl_connect(sk); if (err) { fprintf(stderr, "Failed to connect socket\n"); return err; } grp = genl_ctrl_resolve_grp(sk, "NET_DM", "events"); if (grp < 0) { fprintf(stderr, "Failed to resolve \"events\" multicast group\n"); return grp; } err = nl_socket_add_memberships(sk, grp, NFNLGRP_NONE); if (err) { fprintf(stderr, "Failed to join \"events\" multicast group\n"); return err; } return 0; } $ gcc -I/usr/include/libnl3 -lnl-3 -lnl-genl-3 -o dm_repo dm.c Fixes: 9a8afc8d3962 ("Network Drop Monitor: Adding drop monitor implementation & Netlink protocol") Reported-by: "The UK's National Cyber Security Centre (NCSC)" <security@ncsc.gov.uk> Signed-off-by: Ido Schimmel <idosch@nvidia.com> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20231206213102.1824398-3-idosch@nvidia.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: don't merge dumpit split op for different cmds into single iterJiri Pirko2023-10-241-1/+2
| | | | | | | | | | | | | | Currently, split ops of doit and dumpit are merged into a single iter item when they are subsequent. However, there is no guarantee that the dumpit op is for the same cmd as doit op. Fix this by checking if cmd is the same for both. This problem does not occur in existing families. Signed-off-by: Jiri Pirko <jiri@nvidia.com> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Link: https://lore.kernel.org/r/20231021112711.660606-2-jiri@resnulli.us Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: add a family pointer to struct genl_infoJakub Kicinski2023-08-161-9/+12
| | | | | | | | | | Having family in struct genl_info is quite useful. It cuts down the number of arguments which need to be passed to helpers which already take struct genl_info. Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20230814214723.2924989-7-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: use attrs from struct genl_infoJakub Kicinski2023-08-161-4/+3
| | | | | | | | | | | Since dumps carry struct genl_info now, use the attrs pointer from genl_info and remove the one in struct genl_dumpit_info. Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20230814214723.2924989-6-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: add struct genl_info to struct genl_dumpit_infoJakub Kicinski2023-08-151-2/+14
| | | | | | | | | | | | | Netlink GET implementations must currently juggle struct genl_info and struct netlink_callback, depending on whether they were called from doit or dumpit. Add genl_info to the dump state and populate the fields. This way implementations can simply pass struct genl_info around. Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20230814214723.2924989-5-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: remove userhdr from struct genl_infoJakub Kicinski2023-08-151-1/+0
| | | | | | | | | | | | | Only three families use info->userhdr today and going forward we discourage using fixed headers in new families. So having the pointer to user header in struct genl_info is an overkill. Compute the header pointer at runtime. Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Reviewed-by: Aaron Conole <aconole@redhat.com> Link: https://lore.kernel.org/r/20230814214723.2924989-4-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: push conditional locking into dumpit/doneJakub Kicinski2023-08-151-55/+35
| | | | | | | | | | | | | Add helpers which take/release the genl mutex based on family->parallel_ops. Remove the separation between handling of ops in locked and parallel families. Future patches would make the duplicated code grow even more. Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20230814214723.2924989-2-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: add explicit ordering break check for split opsJiri Pirko2023-07-221-1/+5
| | | | | | | | | | | | | | | Currently, if cmd in the split ops array is of lower value than the previous one, genl_validate_ops() continues to do the checks as if the values are equal. This may result in non-obvious WARN_ON() hit in these check. Instead, check the incorrect ordering explicitly and put a WARN_ON() in case it is broken. Signed-off-by: Jiri Pirko <jiri@nvidia.com> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Link: https://lore.kernel.org/r/20230720111354.562242-1-jiri@resnulli.us Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* netlink: support extack in dump ->start()Jakub Kicinski2023-06-121-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 4a19edb60d02 ("netlink: Pass extack to dump handlers") added extack support to netlink dumps. It was focused on rtnl and since rtnl does not use ->start(), ->done() callbacks it ignored those. Genetlink on the other hand uses ->start() extensively, for parsing and input validation. Pass the extact in via struct netlink_dump_control and link it to cb for the time of ->start(). Both struct netlink_dump_control and extack itself live on the stack so we can't keep the same extack for the duration of the dump. This means that the extack visible in ->start() and each ->dump() callbacks will be different. Corner cases like reporting a warning message in DONE across dump calls are still not supported. We could put the extack (for dumps) in the socket struct, but layering makes it slightly awkward (extack pointer is decided before the DO / DUMP split). The genetlink dump error extacks are now surfaced: $ cli.py --spec netlink/specs/ethtool.yaml --dump channels-get lib.ynl.NlError: Netlink error: Invalid argument nl_len = 64 (48) nl_flags = 0x300 nl_type = 2 error: -22 extack: {'msg': 'request header missing'} Previously extack was missing: $ cli.py --spec netlink/specs/ethtool.yaml --dump channels-get lib.ynl.NlError: Netlink error: Invalid argument nl_len = 36 (20) nl_flags = 0x100 nl_type = 2 error: -22 Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: Use string_is_terminated() helperAndy Shevchenko2023-02-101-2/+2
| | | | | | | | | | | Use string_is_terminated() helper instead of cpecific memchr() call. This shows better the intention of the call. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Simon Horman <simon.horman@corigine.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20230208133153.22528-2-andriy.shevchenko@linux.intel.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: fix single op policy dump when do is presentJakub Kicinski2022-11-101-9/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Jonathan reports crashes when running net-next in Meta's fleet. Stats collection uses ethtool -I which does a per-op policy dump to check if stats are supported. We don't initialize the dumpit information if doit succeeds due to evaluation short-circuiting. The crash may look like this: BUG: kernel NULL pointer dereference, address: 0000000000000cc0 RIP: 0010:netlink_policy_dump_add_policy+0x174/0x2a0 ctrl_dumppolicy_start+0x19f/0x2f0 genl_start+0xe7/0x140 Or we may trigger a warning: WARNING: CPU: 1 PID: 785 at net/netlink/policy.c:87 netlink_policy_dump_get_policy_idx+0x79/0x80 RIP: 0010:netlink_policy_dump_get_policy_idx+0x79/0x80 ctrl_dumppolicy_put_op+0x214/0x360 depending on what garbage we pick up from the stack. Reported-by: Jonathan Lemon <bsd@meta.com> Fixes: 26588edbef60 ("genetlink: support split policies in ctrl_dumppolicy_put_op()") Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Tested-by: Leon Romanovsky <leonro@nvidia.com> Link: https://lore.kernel.org/r/20221109183254.554051-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: correctly begin the iteration over policiesJakub Kicinski2022-11-091-1/+3
| | | | | | | | | | | | | | | | | The return value from genl_op_iter_init() only tells us if there are any policies but to begin the iteration (and therefore load the first entry) we need to call genl_op_iter_next(). Note that it's safe to call genl_op_iter_next() on a family with no ops, it will just return false. This may lead to various crashes, a warning in netlink_policy_dump_get_policy_idx() when policy is not found or.. no problem at all if the kmalloc'ed memory happens to be zeroed. Fixes: b502b3185cd6 ("genetlink: use iterator in the op to policy map dumping") Link: https://lore.kernel.org/r/20221108204128.330287-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: convert control family to split opsJakub Kicinski2022-11-071-4/+13
| | | | | | | | | | | Prove that the split ops work. Sadly we need to keep bug-wards compatibility and specify the same policy for dump as do, even tho we don't parse inputs for the dump. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: allow families to use split ops directlyJakub Kicinski2022-11-071-26/+144
| | | | | | | | | | | | | | | Let families to hook in the new split ops. They are more flexible and should not be much larger than full ops. Each split op is 40B while full op is 48B. Devlink for example has 54 dos and 19 dumps, 2 of the dumps do not have a do -> 56 full commands = 2688B. Split ops would have taken 2920B, so 9% more space while allowing individual per/post doit and per-type policies. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: inline old iteration helpersJakub Kicinski2022-11-071-23/+9
| | | | | | | | | All dumpers use the iterators now, inline the cmd by index stuff into iterator code. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: use iterator in the op to policy map dumpingJakub Kicinski2022-11-071-23/+26
| | | | | | | | | | | | We can't put the full iterator in the struct ctrl_dump_policy_ctx because dump context is statically sized by netlink core. Allocate it dynamically. Rename policy to dump_map to make the logic a little easier to follow. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: add iterator for walking family opsJakub Kicinski2022-11-071-41/+76
| | | | | | | | | | | Subsequent changes will expose split op structures to users, so walking the family ops with just an index will get harder. Add a structured iterator, convert the simple cases. Policy dumping needs more careful conversion. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: inline genl_get_cmd()Jakub Kicinski2022-11-071-18/+12
| | | | | | | | | All callers go via genl_get_cmd_split() now, so rename it to genl_get_cmd() remove the original. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: support split policies in ctrl_dumppolicy_put_op()Jakub Kicinski2022-11-071-22/+33
| | | | | | | | | | | | Pass do and dump versions of the op to ctrl_dumppolicy_put_op() so that it can provide a different policy index for the two. Since we now look at policies, and those are set appropriately there's no need to look at the GENL_DONT_VALIDATE_DUMP flag. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: add policies for both doit and dumpit in ctrl_dumppolicy_start()Jakub Kicinski2022-11-071-10/+38
| | | | | | | | | | | Separate adding doit and dumpit policies for CTRL_CMD_GETPOLICY. This has no effect until we actually allow do and dump to come from different sources as netlink_policy_dump_add_policy() does deduplication. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: check for callback type at op load timeJakub Kicinski2022-11-071-10/+10
| | | | | | | | | | Now that genl_get_cmd_split() is informed what type of callback user is trying to access (do or dump) we can check that this callback is indeed available and return an error early. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: load policy based on validation flagsJakub Kicinski2022-11-071-7/+10
| | | | | | | | | | Set the policy and maxattr pointers based on validation flags. genl_family_rcv_msg_attrs_parse() will do nothing and return NULL if maxattrs is zero, so no behavior change is expected. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: introduce split op representationJakub Kicinski2022-11-071-15/+64
| | | | | | | | | | | | | | | | | | | | | | | | | | | We currently have two forms of operations - small ops and "full" ops (or just ops). The former does not have pointers for some of the less commonly used features (namely dump start/done and policy). The "full" ops, however, still don't contain all the necessary information. In particular the policy is per command ID, while do and dump often accept different attributes. It's also not possible to define different pre_doit and post_doit callbacks for different commands within the family. At the same time a lot of commands do not support dumping and therefore all the dump-related information is wasted space. Create a new command representation which can hold info about a do implementation or a dump implementation, but not both at the same time. Use this new representation on the command execution path (genl_family_rcv_msg) as we either run a do or a dump and don't have to create a "full" op there. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: refactor the cmd <> policy mapping dumpJakub Kicinski2022-11-071-12/+15
| | | | | | | | | | | | The code at the top of ctrl_dumppolicy() dumps mappings between ops and policies. It supports dumping both the entire family and single op if dump is filtered. But both of those cases are handled inside a loop, which makes the logic harder to follow and change. Refactor to split the two cases more clearly. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: limit the use of validation workarounds to old opsJakub Kicinski2022-10-271-0/+2
| | | | | | | | | | | | During review of previous change another thing came up - we should limit the use of validation workarounds to old commands. Don't list the workarounds one by one, as we're rejecting all existing ones. We can deal with the masking in the unlikely event that new flag is added. Link: https://lore.kernel.org/all/6ba9f727e555fd376623a298d5d305ad408c3d47.camel@sipsolutions.net/ Link: https://lore.kernel.org/r/20221026001524.1892202-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: piggy back on resv_op to default to a reject policyJakub Kicinski2022-10-251-0/+23
| | | | | | | | | | | | | | | | | To keep backward compatibility we used to leave attribute parsing to the family if no policy is specified. This becomes tedious as we move to more strict validation. Families must define reject all policies if they don't want any attributes accepted. Piggy back on the resv_start_op field as the switchover point. AFAICT only ethtool has added new commands since the resv_start_op was defined, and it has per-op policies so this should be a no-op. Nonetheless the patch should still go into v6.1 for consistency. Link: https://lore.kernel.org/all/20221019125745.3f2e7659@kernel.org/ Link: https://lore.kernel.org/r/20221021193532.1511293-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: reject use of nlmsg_flags for new commandsJakub Kicinski2022-10-011-1/+31
| | | | | | | | | | | | | | | | | | | | Commit 9c5d03d36251 ("genetlink: start to validate reserved header bytes") introduced extra validation for genetlink headers. We had to gate it to only apply to new commands, to maintain bug-wards compatibility. Use this opportunity (before the new checks make it to Linus's tree) to add more conditions. Validate that Generic Netlink families do not use nlmsg_flags outside of the well-understood set. Link: https://lore.kernel.org/all/20220928073709.1b93b74a@kernel.org/ Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Acked-by: Nikolay Aleksandrov <razor@blackwall.org> Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Reviewed-by: Guillaume Nault <gnault@redhat.com> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Link: https://lore.kernel.org/r/20220929142809.1167546-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* genetlink: start to validate reserved header bytesJakub Kicinski2022-08-291-0/+4
| | | | | | | | | | | | | | | | | We had historically not checked that genlmsghdr.reserved is 0 on input which prevents us from using those precious bytes in the future. One use case would be to extend the cmd field, which is currently just 8 bits wide and 256 is not a lot of commands for some core families. To make sure that new families do the right thing by default put the onus of opting out of validation on existing families. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Acked-by: Paul Moore <paul@paul-moore.com> (NetLabel) Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: hold read cb_lock during iteration of genl_fam_idr in genl_bind()Jiri Pirko2022-08-271-2/+2
| | | | | | | | | | | | | | | | In genl_bind(), currently genl_lock and write cb_lock are taken for iteration of genl_fam_idr and processing of static values stored in struct genl_family. Take just read cb_lock for this task as it is sufficient to guard the idr and the struct against concurrent genl_register/unregister_family() calls. This will allow to run genl command processing in genl_rcv() and mnl_socket_setsockopt(.., NETLINK_ADD_MEMBERSHIP, ..) in parallel. Reported-by: Vikas Gupta <vikas.gupta@broadcom.com> Signed-off-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20220825081940.1283335-1-jiri@resnulli.us Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* net: genl: fix error path memory leak in policy dumpingJakub Kicinski2022-08-181-1/+5
| | | | | | | | | | | | | | If construction of the array of policies fails when recording non-first policy we need to unwind. netlink_policy_dump_add_policy() itself also needs fixing as it currently gives up on error without recording the allocated pointer in the pstate pointer. Reported-by: syzbot+dc54d9ba8153b216cae0@syzkaller.appspotmail.com Fixes: 50a896cf2d6f ("genetlink: properly support per-op policy dumping") Link: https://lore.kernel.org/r/20220816161939.577583-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* net: netlink: Remove unused functionYajun Deng2021-07-301-8/+0
| | | | | | | | lockdep_genl_is_held() and its caller arm not used now, just remove them. Signed-off-by: Yajun Deng <yajun.deng@linux.dev> Link: https://lore.kernel.org/r/20210729074854.8968-1-yajun.deng@linux.dev Signed-off-by: Jakub Kicinski <kuba@kernel.org>
* net: netlink: add the case when nlh is NULLYajun Deng2021-07-271-5/+4
| | | | | | | | Add the case when nlh is NULL in nlmsg_report(), so that the caller doesn't need to deal with this case. Signed-off-by: Yajun Deng <yajun.deng@linux.dev> Signed-off-by: David S. Miller <davem@davemloft.net>
* mptcp: avoid lock_fast usage in accept pathFlorian Westphal2021-02-131-0/+32
| | | | | | | | | | | Once event support is added this may need to allocate memory while msk lock is held with softirqs disabled. Not using lock_fast also allows to do the allocation with GFP_KERNEL. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: allow dumping command-specific policyJakub Kicinski2020-10-031-5/+36
| | | | | | | | | | | | | | | | | | | Right now CTRL_CMD_GETPOLICY can only dump the family-wide policy. Support dumping policy of a specific op. v3: - rebase after per-op policy export and handle that v2: - make cmd U32, just in case. v1: - don't echo op in the output in a naive way, this should make it cleaner to extend the output format for dumping policies for all the commands at once in the future. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Link: https://lore.kernel.org/r/20201001225933.1373426-11-kuba@kernel.org Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: properly support per-op policy dumpingJohannes Berg2020-10-031-10/+92
| | | | | | | | | | | | | | | | | | | Add support for per-op policy dumping. The data is pretty much as before, except that now the assumption that the policy with index 0 is "the" policy no longer holds - you now need to look at the new CTRL_ATTR_OP_POLICY attribute which is a nested attr (indexed by op) containing attributes for do and dump policies. When a single op is requested, the CTRL_ATTR_OP_POLICY will be added in the same way, since do and dump policies may differ. v2: - conditionally advertise per-command policies only if there actually is a policy being used for the do/dump and it's present at all Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: factor skb preparation out of ctrl_dumppolicy()Johannes Berg2020-10-031-6/+19
| | | | | | | | We'll need this later for the per-op policy index dump. Reviewed-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* netlink: rework policy dump to support multiple policiesJohannes Berg2020-10-031-1/+2
| | | | | | | | | | | | | | | | | Rework the policy dump code a bit to support adding multiple policies to a single dump, in order to e.g. support per-op policies in generic netlink. v2: - move kernel-doc to implementation [Jakub] - squash the first patch to not flip-flop on the prototype [Jakub] - merge netlink_policy_dump_get_policy_idx() with the old get_policy_idx() we already had - rebase without Jakub's patch to have per-op dump Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: switch control commands to per-op policiesJakub Kicinski2020-10-031-3/+11
| | | | | | | | | | | | | | In preparation for adding a new attribute to CTRL_CMD_GETPOLICY split the policies for getpolicy and getfamily apart. This will cause a slight user-visible change in that dumping the policies will switch from per family to per op, but supposedly sniffer-type applications (which are the main use case for policy dumping thus far) should support both, anyway. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: use parsed attrs in dumppolicyJakub Kicinski2020-10-031-7/+2
| | | | | | | | | | Attributes are already parsed based on the policy specified in the family and ready-to-use in info->attrs. No need to call genlmsg_parse() again. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: bring back per op policyJakub Kicinski2020-10-031-5/+13
| | | | | | | | | | | | | | Add policy to the struct genl_ops structure, this time with maxattr, so it can be used properly. Propagate .policy and .maxattr from the family in genl_get_cmd() if needed, this way the rest of the code does not have to worry if the policy is per op or global. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: use .start callback for dumppolicyJakub Kicinski2020-10-031-24/+24
| | | | | | | | | | The structure of ctrl_dumppolicy() is clearly split into init and dumping. Move the init to a .start callback for clarity, it's a more idiomatic netlink dump code structure. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: add a structure for dump stateJakub Kicinski2020-10-031-10/+18
| | | | | | | | | | | | | | | | | | | | | | | Whenever netlink dump uses more than 2 cb->args[] entries code gets hard to read. We're about to add more state to ctrl_dumppolicy() so create a structure. Since the structure is typed and clearly named we can remove the local fam_id variable and use ctx->fam_id directly. v3: - rebase onto explicit free fix v1: - s/nl_policy_dump/netlink_policy_dump_state/ - forward declare struct netlink_policy_dump_state, and move from passing unsigned long to actual pointer type - add build bug on - u16 fam_id - s/args/ctx/ Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: David S. Miller <davem@davemloft.net>