Re: [PATCH bpf-next v4 1/2] bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules

[Viktor Malik <vmalik@redhat.com>]

On 12/12/22 13:59, 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 from it and search for the function address in
> the module.
> 
> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> ---
>   kernel/bpf/verifier.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a5255a0dcbb6..d646c5263bc5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -24,6 +24,7 @@
>   #include <linux/bpf_lsm.h>
>   #include <linux/btf_ids.h>
>   #include <linux/poison.h>
> +#include "../module/internal.h"

Looks like there's a number of errors reported by test robot due to
including "../module/internal.h" (mostly unrelated to this patchset).
I'm not sure how to approach those - move find_kallsyms_symbol_value to
include/linux/module.h or just ignore the errors?

For both cases, a trivial find_kallsyms_symbol_value for
!CONFIG_KALLSYMS will be necessary.

>   
>   #include "disasm.h"
>   
> @@ -16478,6 +16479,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>   	const char *tname;
>   	struct btf *btf;
>   	long addr = 0;
> +	struct module *mod;
>   
>   	if (!btf_id) {
>   		bpf_log(log, "Tracing programs must provide btf_id\n");
> @@ -16645,7 +16647,19 @@ 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)) {
> +				preempt_disable();
> +				mod = btf_try_get_module(btf);
> +				if (mod) {
> +					addr = find_kallsyms_symbol_value(mod, tname);
> +					module_put(mod);
> +				} else {
> +					addr = 0;
> +				}
> +				preempt_enable();
> +			} else {
> +				addr = kallsyms_lookup_name(tname);
> +			}
>   			if (!addr) {
>   				bpf_log(log,
>   					"The address of function %s cannot be found\n",