From: Ihor Solodrai <[email protected]> bpf_core_find_cands() calls bpf_get_btf_vmlinux() with cand_cache_mutex held, from btf_get_ptr_to_btf_id() and bpf_core_apply(). Once the vmlinux BTF is parsed on first use (later in this series), parsing also parses the module BTFs kept until then, under btf_module_mutex, while a module going away takes btf_module_mutex and then cand_cache_mutex (purge_cand_cache()). That would be an ABBA deadlock:
cand_cache_mutex -> btf_vmlinux_lock -> btf_module_mutex btf_module_mutex -> cand_cache_mutex Have both callers fetch the BTF before they take cand_cache_mutex, and make bpf_core_find_cands() only peek with bpf_peek_btf_vmlinux(). The ordering was found by design review. With the whole series applied and the fetch moved back under the mutex, PROVE_LOCKING reports this cycle at the first rmmod of a BTF module after a kernel CO-RE load parsed the BTF; with this patch the same sequence is clean. No functional change: the BTF is parsed at the same points, just before the mutex is taken; a broken BTF fails the load with the error of its parse, and bpf_core_find_cands() handles a missing BTF as before. TODO-NEEDS-IHOR: Signed-off-by: Ihor Solodrai <[email protected]> [pass the error of the fetch on instead of -EINVAL from the peek] Signed-off-by: Jay Wang <[email protected]> --- kernel/bpf/btf.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 59d6f44cbe17..70adc52f5d7c 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -8596,6 +8596,7 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx, .btf = btf, .log = log, }; + struct btf *vmlinux_btf; u32 kern_type_id, type_id; int err = 0; @@ -8607,6 +8608,10 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx, t = btf_type_by_id(btf, t->type); } + /* parse it, if this is its first use, before taking cand_cache_mutex */ + vmlinux_btf = bpf_get_btf_vmlinux(); + if (IS_ERR(vmlinux_btf)) + return PTR_ERR(vmlinux_btf); mutex_lock(&cand_cache_mutex); cc = bpf_core_find_cands(&ctx, type_id); if (IS_ERR(cc)) { @@ -10610,7 +10615,8 @@ bpf_core_find_cands(struct bpf_core_ctx *ctx, u32 local_type_id) const char *name; int id; - main_btf = bpf_get_btf_vmlinux(); + /* callers parse it before taking cand_cache_mutex, see bpf_core_apply() */ + main_btf = bpf_peek_btf_vmlinux(); if (IS_ERR(main_btf)) return ERR_CAST(main_btf); if (!main_btf) @@ -10713,8 +10719,18 @@ int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo, if (need_cands) { struct bpf_cand_cache *cc; + struct btf *main_btf; int i; + /* + * Parsing the vmlinux BTF takes btf_module_mutex, which is + * taken before cand_cache_mutex when a module goes away. + */ + main_btf = bpf_get_btf_vmlinux(); + if (IS_ERR(main_btf)) { + kfree(specs); + return PTR_ERR(main_btf); + } mutex_lock(&cand_cache_mutex); cc = bpf_core_find_cands(ctx, relo->type_id); if (IS_ERR(cc)) { -- 2.47.3
