summaryrefslogtreecommitdiffstats
path: root/net/netlink/genetlink.c
Commit message (Collapse)AuthorAgeFilesLines
* 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-241-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-09-301-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-261-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-121-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-021-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-021-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-021-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-021-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-021-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>
* genetlink: add small version of opsJakub Kicinski2020-10-021-32/+95
| | | | | | | | | | | | | | | | | | We want to add maxattr and policy back to genl_ops, to enable dumping per command policy to user space. This, however, would cause bloat for all the families with global policies. Introduce smaller version of ops (half the size of genl_ops). Translate these smaller ops into a full blown struct before use in the core. v1: - use struct assignment - put a full copy of the op in struct genl_dumpit_info - s/light/small/ Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: David S. Miller <davem@davemloft.net>
* netlink: fix policy dump leakJohannes Berg2020-10-021-1/+8
| | | | | | | | | | | | [ Upstream commit a95bc734e60449e7b073ff7ff70c35083b290ae9 ] If userspace doesn't complete the policy dump, we leak the allocated state. Fix this. Fixes: d07dcf9aadd6 ("netlink: add infrastructure to expose policies to userspace") Signed-off-by: Johannes Berg <johannes.berg@intel.com> Reviewed-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: netlink: delete repeated wordsRandy Dunlap2020-08-241-1/+1
| | | | | | | | | Drop duplicated words in net/netlink/. Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
* Merge tag 'thermal-v5.9-rc1' of ↵Linus Torvalds2020-08-061-1/+1
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux Pull thermal updates from Daniel Lezcano: - Add support to enable/disable the thermal zones resulting on core code and drivers cleanup (Andrzej Pietrasiewicz) - Add generic netlink support for userspace notifications: events, temperature and discovery commands (Daniel Lezcano) - Fix redundant initialization for a ret variable (Colin Ian King) - Remove the clock cooling code as it is used nowhere (Amit Kucheria) - Add the rcar_gen3_thermal's r8a774e1 support (Marian-Cristian Rotariu) - Replace all references to thermal.txt in the documentation to the corresponding yaml files (Amit Kucheria) - Add maintainer entry for the IPA (Lukasz Luba) - Add support for MSM8939 for the tsens (Shawn Guo) - Update power allocator and devfreq cooling to SPDX licensing (Lukasz Luba) - Add Cannon Lake Low Power PCH support (Sumeet Pawnikar) - Add tsensor support for V2 mediatek thermal system (Henry Yen) - Fix thermal zone lookup by ID for the core code (Thierry Reding) * tag 'thermal-v5.9-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux: (40 commits) thermal: intel: intel_pch_thermal: Add Cannon Lake Low Power PCH support thermal: mediatek: Add tsensor support for V2 thermal system thermal: mediatek: Prepare to add support for other platforms thermal: Update power allocator and devfreq cooling to SPDX licensing MAINTAINERS: update entry to thermal governors file name prefixing thermal: core: Add thermal zone enable/disable notification thermal: qcom: tsens-v0_1: Add support for MSM8939 dt-bindings: tsens: qcom: Document MSM8939 compatible thermal: core: Fix thermal zone lookup by ID thermal: int340x: processor_thermal: fix: update Jasper Lake PCI id thermal: imx8mm: Support module autoloading thermal: ti-soc-thermal: Fix reversed condition in ti_thermal_expose_sensor() MAINTAINERS: Add maintenance information for IPA thermal: rcar_gen3_thermal: Do not shadow thcode variable dt-bindings: thermal: Get rid of thermal.txt and replace references thermal: core: Move initialization after core initcall thermal: netlink: Improve the initcall ordering net: genetlink: Move initialization to core_initcall thermal: rcar_gen3_thermal: Add r8a774e1 support thermal/drivers/clock_cooling: Remove clock_cooling code ...
| * net: genetlink: Move initialization to core_initcallDaniel Lezcano2020-07-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The generic netlink is initialized far after the netlink protocol itself at subsys_initcall. The devlink is initialized at the same level, but after, as shown by a disassembly of the vmlinux: [ ... ] 374 ffff8000115f22c0 <__initcall_devlink_init4>: 375 ffff8000115f22c4 <__initcall_genl_init4>: [ ... ] The function devlink_init() calls genl_register_family() before the generic netlink subsystem is initialized. As the generic netlink initcall level is set since 2005, it seems that was not a problem, but now we have the thermal framework initialized at the core_initcall level which creates the generic netlink family and sends a notification which leads to a subtle memory corruption only detectable when the CONFIG_INIT_ON_ALLOC_DEFAULT_ON option is set with the earlycon at init time. The thermal framework needs to be initialized early in order to begin the mitigation as soon as possible. Moving it to postcore_initcall is acceptable. This patch changes the initialization level for the generic netlink family to the core_initcall and comes after the netlink protocol initialization. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Acked-by: David S. Miller <davem@davemloft.net> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org> Link: https://lore.kernel.org/r/20200715074120.8768-1-daniel.lezcano@linaro.org
* | genetlink: remove genl_bindSean Tranchetti2020-07-011-49/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A potential deadlock can occur during registering or unregistering a new generic netlink family between the main nl_table_lock and the cb_lock where each thread wants the lock held by the other, as demonstrated below. 1) Thread 1 is performing a netlink_bind() operation on a socket. As part of this call, it will call netlink_lock_table(), incrementing the nl_table_users count to 1. 2) Thread 2 is registering (or unregistering) a genl_family via the genl_(un)register_family() API. The cb_lock semaphore will be taken for writing. 3) Thread 1 will call genl_bind() as part of the bind operation to handle subscribing to GENL multicast groups at the request of the user. It will attempt to take the cb_lock semaphore for reading, but it will fail and be scheduled away, waiting for Thread 2 to finish the write. 4) Thread 2 will call netlink_table_grab() during the (un)registration call. However, as Thread 1 has incremented nl_table_users, it will not be able to proceed, and both threads will be stuck waiting for the other. genl_bind() is a noop, unless a genl_family implements the mcast_bind() function to handle setting up family-specific multicast operations. Since no one in-tree uses this functionality as Cong pointed out, simply removing the genl_bind() function will remove the possibility for deadlock, as there is no attempt by Thread 1 above to take the cb_lock semaphore. Fixes: c380d9a7afff ("genetlink: pass multicast bind/unbind to families") Suggested-by: Cong Wang <xiyou.wangcong@gmail.com> Acked-by: Johannes Berg <johannes.berg@intel.com> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Sean Tranchetti <stranche@codeaurora.org> Signed-off-by: David S. Miller <davem@davemloft.net>
* | genetlink: get rid of family->attrbufCong Wang2020-06-291-35/+13
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | genl_family_rcv_msg_attrs_parse() reuses the global family->attrbuf when family->parallel_ops is false. However, family->attrbuf is not protected by any lock on the genl_family_rcv_msg_doit() code path. This leads to several different consequences, one of them is UAF, like the following: genl_family_rcv_msg_doit(): genl_start(): genl_family_rcv_msg_attrs_parse() attrbuf = family->attrbuf __nlmsg_parse(attrbuf); genl_family_rcv_msg_attrs_parse() attrbuf = family->attrbuf __nlmsg_parse(attrbuf); info->attrs = attrs; cb->data = info; netlink_unicast_kernel(): consume_skb() genl_lock_dumpit(): genl_dumpit_info(cb)->attrs Note family->attrbuf is an array of pointers to the skb data, once the skb is freed, any dereference of family->attrbuf will be a UAF. Maybe we could serialize the family->attrbuf with genl_mutex too, but that would make the locking more complicated. Instead, we can just get rid of family->attrbuf and always allocate attrbuf from heap like the family->parallel_ops==true code path. This may add some performance overhead but comparing with taking the global genl_mutex, it still looks better. Fixes: 75cdbdd08900 ("net: ieee802154: have genetlink code to parse the attrs during dumpit") Fixes: 057af7071344 ("net: tipc: have genetlink code to parse the attrs during dumpit") Reported-and-tested-by: syzbot+3039ddf6d7b13daf3787@syzkaller.appspotmail.com Reported-and-tested-by: syzbot+80cad1e3cb4c41cde6ff@syzkaller.appspotmail.com Reported-and-tested-by: syzbot+736bcbcb11b60d0c0792@syzkaller.appspotmail.com Reported-and-tested-by: syzbot+520f8704db2b68091d44@syzkaller.appspotmail.com Reported-and-tested-by: syzbot+c96e4dfb32f8987fdeed@syzkaller.appspotmail.com Cc: Jiri Pirko <jiri@mellanox.com> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: clean up family attributes allocationsCong Wang2020-06-121-16/+12
| | | | | | | | | | | | | | | | | | | | genl_family_rcv_msg_attrs_parse() and genl_family_rcv_msg_attrs_free() take a boolean parameter to determine whether allocate/free the family attrs. This is unnecessary as we can just check family->parallel_ops. More importantly, callers would not need to worry about pairing these parameters correctly after this patch. And this fixes a memory leak, as after commit c36f05559104 ("genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()") we call genl_family_rcv_msg_attrs_parse() for both parallel and non-parallel cases. Fixes: c36f05559104 ("genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()") Reported-by: Ido Schimmel <idosch@idosch.org> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Tested-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()Cong Wang2020-06-041-36/+58
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are two kinds of memory leaks in genl_family_rcv_msg_dumpit(): 1. Before we call ops->start(), whenever an error happens, we forget to free the memory allocated in genl_family_rcv_msg_dumpit(). 2. When ops->start() fails, the 'info' has been already installed on the per socket control block, so we should not free it here. More importantly, nlk->cb_running is still false at this point, so netlink_sock_destruct() cannot free it either. The first kind of memory leaks is easier to resolve, but the second one requires some deeper thoughts. After reviewing how netfilter handles this, the most elegant solution I find is just to use a similar way to allocate the memory, that is, moving memory allocations from caller into ops->start(). With this, we can solve both kinds of memory leaks: for 1), no memory allocation happens before ops->start(); for 2), ops->start() handles its own failures and 'info' is installed to the socket control block only when success. The only ugliness here is we have to pass all local variables on stack via a struct, but this is not hard to understand. Alternatively, we can introduce a ops->free() to solve this too, but it is overkill as only genetlink has this problem so far. Fixes: 1927f41a22a0 ("net: genetlink: introduce dump info struct to be available during dumpit op") Reported-by: syzbot+21f04f481f449c8db840@syzkaller.appspotmail.com Cc: "Jason A. Donenfeld" <Jason@zx2c4.com> Cc: Florian Westphal <fw@strlen.de> Cc: Pablo Neira Ayuso <pablo@netfilter.org> Cc: Jiri Pirko <jiri@mellanox.com> Cc: YueHaibing <yuehaibing@huawei.com> Cc: Shaochun Chen <cscnull@gmail.com> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* netlink: add infrastructure to expose policies to userspaceJohannes Berg2020-04-301-0/+78
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add, and use in generic netlink, helpers to dump out a netlink policy to userspace, including all the range validation data, nested policies etc. This lets userspace discover what the kernel understands. For families/commands other than generic netlink, the helpers need to be used directly in an appropriate command, or we can add some infrastructure (a new netlink family) that those can register their policies with for introspection. I'm not that familiar with non-generic netlink, so that's left out for now. The data exposed to userspace also includes min and max length for binary/string data, I've done that instead of letting the userspace tools figure out whether min/max is intended based on the type so that we can extend this later in the kernel, we might want to just use the range data for example. Because of this, I opted to not directly expose the NLA_* values, even if some of them are already exposed via BPF, as with min/max length we don't need to have different types here for NLA_BINARY/NLA_MIN_LEN/NLA_EXACT_LEN, we just make them all NL_ATTR_TYPE_BINARY with min/max length optionally set. Similarly, we don't really need NLA_MSECS, and perhaps can remove it in the future - but not if we encode it into the userspace API now. It gets mapped to NL_ATTR_TYPE_U64 here. Note that the exposing here corresponds to the strict policy interpretation, and NLA_UNSPEC items are omitted entirely. To get those, change them to NLA_MIN_LEN which behaves in exactly the same way, but is exposed. Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: genetlink: return the error code when attribute parsing fails.Paolo Abeni2020-02-221-2/+3
| | | | | | | | | | | | | | Currently if attribute parsing fails and the genl family does not support parallel operation, the error code returned by __nlmsg_parse() is discarded by genl_family_rcv_msg_attrs_parse(). Be sure to report the error for all genl families. Fixes: c10e6cf85e7d ("net: genetlink: push attrbuf allocation and parsing to a separate function") Fixes: ab5b526da048 ("net: genetlink: always allocate separate attrs for dumpit ops") Signed-off-by: Paolo Abeni <pabeni@redhat.com> Reviewed-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* genetlink: do not parse attributes for families with zero maxattrMichal Kubecek2019-10-131-6/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit c10e6cf85e7d ("net: genetlink: push attrbuf allocation and parsing to a separate function") moved attribute buffer allocation and attribute parsing from genl_family_rcv_msg_doit() into a separate function genl_family_rcv_msg_attrs_parse() which, unlike the previous code, calls __nlmsg_parse() even if family->maxattr is 0 (i.e. the family does its own parsing). The parser error is ignored and does not propagate out of genl_family_rcv_msg_attrs_parse() but an error message ("Unknown attribute type") is set in extack and if further processing generates no error or warning, it stays there and is interpreted as a warning by userspace. Dumpit requests are not affected as genl_family_rcv_msg_dumpit() bypasses the call of genl_family_rcv_msg_attrs_parse() if family->maxattr is zero. Move this logic inside genl_family_rcv_msg_attrs_parse() so that we don't have to handle it in each caller. v3: put the check inside genl_family_rcv_msg_attrs_parse() v2: adjust also argument of genl_family_rcv_msg_attrs_free() Fixes: c10e6cf85e7d ("net: genetlink: push attrbuf allocation and parsing to a separate function") Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Acked-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: genetlink: always allocate separate attrs for dumpit opsJiri Pirko2019-10-081-11/+17
| | | | | | | | | | | | | | | | | | Individual dumpit ops (start, dumpit, done) are locked by genl_lock if !family->parallel_ops. However, multiple genl_family_rcv_msg_dumpit() calls may in in flight in parallel. Each has a separate struct genl_dumpit_info allocated but they share the same family->attrbuf. Fix this by allocating separate memory for attrs for dumpit ops, for non-parallel_ops (for parallel_ops it is done already). Reported-by: syzbot+495688b736534bb6c6ad@syzkaller.appspotmail.com Reported-by: syzbot+ff59dc711f2cff879a05@syzkaller.appspotmail.com Reported-by: syzbot+dbe02e13bcce52bcf182@syzkaller.appspotmail.com Reported-by: syzbot+9cb7edb2906ea1e83006@syzkaller.appspotmail.com Fixes: bf813b0afeae ("net: genetlink: parse attrs and store in contect info struct during dumpit") Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
* net: genetlink: remove unused genl_family_attrbuf()Jiri Pirko2019-10-061-19/+0
| | | | | | | genl_family_attrbuf() function is no longer used by anyone, so remove it. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: genetlink: parse attrs and store in contect info struct during dumpitJiri Pirko2019-10-061-17/+22
| | | | | | | | | | | Extend the dumpit info struct for attrs. Instead of existing attribute validation do parse them and save in the info struct. Caller can benefit from this and does not have to do parse itself. In order to properly free attrs, genl_family pointer needs to be added to dumpit info struct as well. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: genetlink: push attrbuf allocation and parsing to a separate functionJiri Pirko2019-10-061-22/+45
| | | | | | | | | | | | To be re-usable by dumpit as well, push the code that is taking care of attrbuf allocation and parting from doit into separate function. Introduce a helper to free the buffer too. Check family->maxattr too before calling kfree() to be symmetrical with the allocation check. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: genetlink: introduce dump info struct to be available during dumpit opJiri Pirko2019-10-061-9/+38
| | | | | | | | | | | Currently the cb->data is taken by ops during non-parallel dumping. Introduce a new structure genl_dumpit_info and store the ops there. Distribute the info to both non-parallel and parallel dumping. Also add a helper genl_dumpit_info() to easily get the info structure in the dumpit callback from cb. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: genetlink: push doit/dumpit code from genl_family_rcv_msgJiri Pirko2019-10-061-77/+96
| | | | | | | | | | | Currently the function genl_family_rcv_msg() is quite big. Since it is quite convenient, push code that is related to doit and dumpit ops into separate functions. Do small changes on the way, like rc/err unification, NULL check etc. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-24 21:01:36 +0000