On 3/10/23 20:16, Luis Chamberlain wrote:
Please add linux-modules in the future. My review below.
Sorry for missing that, I'll add it next time.
On Fri, Mar 10, 2023 at 08:40:59AM +0100, Viktor Malik wrote:
[snip]
My review of the critical part below.diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 45a082284464..3905bb20b9a1 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -18432,8 +18434,17 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, else addr = (long) tgt_prog->aux->func[subprog]->bpf_func; } else { - addr = kallsyms_lookup_name(tname); + if (btf_is_module(btf)) { + mod = btf_try_get_module(btf); + if (mod) + addr = find_kallsyms_symbol_value(mod, tname); + else + addr = 0; + } else { + addr = kallsyms_lookup_name(tname); + } if (!addr) { + module_put(mod); bpf_log(log, "The address of function %s cannot be found\n", tname);If btf_modules linked list is ensured to not remove the btf module during this operation, sure this is safe, as per the new guidelines I've posted for try_module_get() this seems to be using try_module_get() using the implied protection.
I believe that is the case. btf_try_get_module checks the BTF_F_MODULE_LIVE flag before calling try_module_get and the flag is set only when the module notifier callback is called with MODULE_STATE_LIVE. In addition, all BTF module operations are called under the same mutex, so the module cannot be removed in-between.
Please review the docs. *If* it respects that usage then feel free to add: Reviewed-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
Thanks for the review. Unless there are more change requests, I'll leave it up to the maintainers to add the tag. Viktor
Luis
