From: Kumar Kartikeya Dwivedi Date: Sat, 9 Oct 2021 04:09:00 +0000 (+0530) Subject: bpf: Silence Coverity warning for find_kfunc_desc_btf X-Git-Tag: v6.6.17~8953^2~2^2~55 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=588cd7ef5382eb89d09a64ee9ba85e4f9a5b0bcb;p=platform%2Fkernel%2Flinux-rpi.git bpf: Silence Coverity warning for find_kfunc_desc_btf The helper function returns a pointer that in the failure case encodes an error in the struct btf pointer. The current code lead to Coverity warning about the use of the invalid pointer: *** CID 1507963: Memory - illegal accesses (USE_AFTER_FREE) /kernel/bpf/verifier.c: 1788 in find_kfunc_desc_btf() 1782 return ERR_PTR(-EINVAL); 1783 } 1784 1785 kfunc_btf = __find_kfunc_desc_btf(env, offset, btf_modp); 1786 if (IS_ERR_OR_NULL(kfunc_btf)) { 1787 verbose(env, "cannot find module BTF for func_id %u\n", func_id); >>> CID 1507963: Memory - illegal accesses (USE_AFTER_FREE) >>> Using freed pointer "kfunc_btf". 1788 return kfunc_btf ?: ERR_PTR(-ENOENT); 1789 } 1790 return kfunc_btf; 1791 } 1792 return btf_vmlinux ?: ERR_PTR(-ENOENT); 1793 } Daniel suggested the use of ERR_CAST so that the intended use is clear to Coverity, but on closer look it seems that we never return NULL from the helper. Andrii noted that since __find_kfunc_desc_btf already logs errors for all cases except btf_get_by_fd, it is much easier to add logging for that and remove the IS_ERR check altogether, returning directly from it. Suggested-by: Andrii Nakryiko Signed-off-by: Kumar Kartikeya Dwivedi Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20211009040900.803436-1-memxor@gmail.com --- diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 20900a1..21cdff3 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1727,8 +1727,10 @@ static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env, return ERR_PTR(-EFAULT); btf = btf_get_by_fd(btf_fd); - if (IS_ERR(btf)) + if (IS_ERR(btf)) { + verbose(env, "invalid module BTF fd specified\n"); return btf; + } if (!btf_is_module(btf)) { verbose(env, "BTF fd for kfunc is not a module BTF\n"); @@ -1771,8 +1773,6 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, u32 func_id, s16 offset, struct module **btf_modp) { - struct btf *kfunc_btf; - if (offset) { if (offset < 0) { /* In the future, this can be allowed to increase limit @@ -1782,12 +1782,7 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, return ERR_PTR(-EINVAL); } - kfunc_btf = __find_kfunc_desc_btf(env, offset, btf_modp); - if (IS_ERR_OR_NULL(kfunc_btf)) { - verbose(env, "cannot find module BTF for func_id %u\n", func_id); - return kfunc_btf ?: ERR_PTR(-ENOENT); - } - return kfunc_btf; + return __find_kfunc_desc_btf(env, offset, btf_modp); } return btf_vmlinux ?: ERR_PTR(-ENOENT); }