On Mon, Nov 28, 2022 at 08:26:29AM +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>
> ---
> include/linux/btf.h | 1 +
> kernel/bpf/btf.c | 5 +++++
> kernel/bpf/verifier.c | 9 ++++++++-
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 9ed00077db6e..bdbf3eb7083d 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -187,6 +187,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 1a59cc7ad730..211fcbb7649d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7192,6 +7192,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;
> +}
> +
> enum {
> BTF_MODULE_F_LIVE = (1 << 0),
> };
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9528a066cfa5..acbe62a73559 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -16343,7 +16343,14 @@ 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)) {
> + char tmodname[MODULE_NAME_LEN + KSYM_NAME_LEN + 1];
looks good.. would be nice to have module_kallsyms lookup function that
takes module name and symbol separately so we won't waste stack on that..
especially when module_kallsyms_lookup_name just separates it back again
and does module lookup.. but not sure how much pain it'd be
jirka
> + snprintf(tmodname, sizeof(tmodname), "%s:%s",
> + btf_module_name(btf), tname);
> + addr = module_kallsyms_lookup_name(tmodname);
> + }
> + else
> + addr = kallsyms_lookup_name(tname);
> if (!addr) {
> bpf_log(log,
> "The address of function %s cannot be found\n",
> --
> 2.38.1
>
