summaryrefslogtreecommitdiff
path: root/net/netlink/genetlink.c
AgeCommit message (Collapse)AuthorFilesLines
2023-10-24genetlink: don't merge dumpit split op for different cmds into single iterJiri Pirko1-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>
2023-08-16genetlink: add a family pointer to struct genl_infoJakub Kicinski1-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>
2023-08-16genetlink: use attrs from struct genl_infoJakub Kicinski1-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>
2023-08-16genetlink: add struct genl_info to struct genl_dumpit_infoJakub Kicinski1-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>
2023-08-16genetlink: remove userhdr from struct genl_infoJakub Kicinski1-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>
2023-08-16genetlink: push conditional locking into dumpit/doneJakub Kicinski1-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>
2023-07-22genetlink: add explicit ordering break check for split opsJiri Pirko1-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>
2023-06-12netlink: support extack in dump ->start()Jakub Kicinski1-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>
2023-02-10genetlink: Use string_is_terminated() helperAndy Shevchenko1-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>
2022-11-11genetlink: fix single op policy dump when do is presentJakub Kicinski1-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>
2022-11-09genetlink: correctly begin the iteration over policiesJakub Kicinski1-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>
2022-11-07genetlink: convert control family to split opsJakub Kicinski1-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>
2022-11-07genetlink: allow families to use split ops directlyJakub Kicinski1-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>
2022-11-07genetlink: inline old iteration helpersJakub Kicinski1-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>
2022-11-07genetlink: use iterator in the op to policy map dumpingJakub Kicinski1-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>
2022-11-07genetlink: add iterator for walking family opsJakub Kicinski1-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>
2022-11-07genetlink: inline genl_get_cmd()Jakub Kicinski1-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>
2022-11-07genetlink: support split policies in ctrl_dumppolicy_put_op()Jakub Kicinski1-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>
2022-11-07genetlink: add policies for both doit and dumpit in ctrl_dumppolicy_start()Jakub Kicinski1-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>
2022-11-07genetlink: check for callback type at op load timeJakub Kicinski1-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>
2022-11-07genetlink: load policy based on validation flagsJakub Kicinski1-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>
2022-11-07genetlink: introduce split op representationJakub Kicinski1-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>
2022-11-07genetlink: refactor the cmd <> policy mapping dumpJakub Kicinski1-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>
2022-10-27genetlink: limit the use of validation workarounds to old opsJakub Kicinski1-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>
2022-10-25genetlink: piggy back on resv_op to default to a reject policyJakub Kicinski1-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>
2022-10-01genetlink: reject use of nlmsg_flags for new commandsJakub Kicinski1-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>
2022-08-29genetlink: start to validate reserved header bytesJakub Kicinski1-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>
2022-08-27genetlink: hold read cb_lock during iteration of genl_fam_idr in genl_bind()Jiri Pirko1-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>
2022-08-18net: genl: fix error path memory leak in policy dumpingJakub Kicinski1-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>
2021-07-30net: netlink: Remove unused functionYajun Deng1-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>
2021-07-27net: netlink: add the case when nlh is NULLYajun Deng1-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>
2021-02-13mptcp: avoid lock_fast usage in accept pathFlorian Westphal1-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>
2020-10-04genetlink: allow dumping command-specific policyJakub Kicinski1-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>
2020-10-04genetlink: properly support per-op policy dumpingJohannes Berg1-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>
2020-10-04genetlink: factor skb preparation out of ctrl_dumppolicy()Johannes Berg1-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>
2020-10-04netlink: rework policy dump to support multiple policiesJohannes Berg1-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>
2020-10-03genetlink: switch control commands to per-op policiesJakub Kicinski1-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>
2020-10-03genetlink: use parsed attrs in dumppolicyJakub Kicinski1-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>
2020-10-03genetlink: bring back per op policyJakub Kicinski1-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>
2020-10-03genetlink: use .start callback for dumppolicyJakub Kicinski1-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>
2020-10-03genetlink: add a structure for dump stateJakub Kicinski1-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>
2020-10-03genetlink: add small version of opsJakub Kicinski1-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>
2020-10-02netlink: fix policy dump leakJohannes Berg1-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>
2020-08-25net: netlink: delete repeated wordsRandy Dunlap1-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>
2020-08-07Merge tag 'thermal-v5.9-rc1' of ↵Linus Torvalds1-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 ...
2020-07-21net: genetlink: Move initialization to core_initcallDaniel Lezcano1-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
2020-07-02genetlink: remove genl_bindSean Tranchetti1-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>
2020-06-30genetlink: get rid of family->attrbufCong Wang1-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>
2020-06-13genetlink: clean up family attributes allocationsCong Wang1-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>
2020-06-05genetlink: fix memory leaks in genl_family_rcv_msg_dumpit()Cong Wang1-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>
generated by cgit v1.2.3 (git 2.39.0) at 2024-11-18 09:01:25 +0300