On Mon, Dec 05, 2022 at 04:26:05PM +0100, Viktor Malik wrote:
> When attaching fentry/fexit/fmod_ret/lsm to a function located in a
> module without specifying the target program, the verifier tries to find
> the address to attach to in kallsyms. This is always done by searching
> the entire kallsyms, not respecting the module in which the function is
> located.
>
> This approach causes an incorrect attachment address to be computed if
> the function to attach to is shadowed by a function of the same name
> located earlier in kallsyms.
>
> Since the attachment must contain the BTF of the program to attach to,
> we may extract the module name from it (if the attach target is a
> module) and search for the function address in the correct module.
>
> Signed-off-by: Viktor Malik <vmalik@xxxxxxxxxx>
> Acked-by: Hao Luo <haoluo@xxxxxxxxxx>
> ---
> include/linux/btf.h | 1 +
> kernel/bpf/btf.c | 5 +++++
> kernel/bpf/verifier.c | 5 ++++-
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index cbd6e4096f8c..b7b791d1f3d6 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -188,6 +188,7 @@ u32 btf_obj_id(const struct btf *btf);
> bool btf_is_kernel(const struct btf *btf);
> bool btf_is_module(const struct btf *btf);
> struct module *btf_try_get_module(const struct btf *btf);
> +const char *btf_module_name(const struct btf *btf);
> u32 btf_nr_types(const struct btf *btf);
> bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
> const struct btf_member *m,
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index c80bd8709e69..f78e8060efa6 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7208,6 +7208,11 @@ bool btf_is_module(const struct btf *btf)
> return btf->kernel_btf && strcmp(btf->name, "vmlinux") != 0;
> }
>
> +const char *btf_module_name(const struct btf *btf)
> +{
> + return btf->name;
> +}
It feels that btf->name is leaking a bit of implementation detail.
How about doing:
struct module *btf_find_module(const struct btf *btf)
{
reutrn find_module(btf->name);
}
> +
> enum {
> BTF_MODULE_F_LIVE = (1 << 0),
> };
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1d51bd9596da..0c533db51f92 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -16483,7 +16483,10 @@ 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))
> + addr = kallsyms_lookup_name_in_module(btf_module_name(btf), tname);
and use find_kallsyms_symbol_value() here
(with preempt_disable dance).
There won't be a need for patch 1 too.
wdyt?
> + else
> + addr = kallsyms_lookup_name(tname);
> if (!addr) {
> bpf_log(log,
> "The address of function %s cannot be found\n",
> --
> 2.38.1
>
