Re: [PATCH bpf] bpf: make DEBUG_INFO_BTF_MODULES selectable independently

[sdf@google.com]

On 10/04, Stanislav Fomichev wrote:
> We're having an issue where we have a recent clang that seems
> to generate kind_flag for enums (aka, adding signed/unsigned).
> Trying to install a module on a kernel that doesn't have commit
> 6089fb325cf7 ("bpf: Add btf enum64 support") returns the following:

> [    3.176954] BPF:Invalid btf_info kind_flag

> The enum that it complains about doesn't seem to have anything special
> except having a sign:

> [1721] ENUM 'perf_event_state' encoding=SIGNED size=4 vlen=6
>          'PERF_EVENT_STATE_DEAD' val=-4
>          'PERF_EVENT_STATE_EXIT' val=-3
>          'PERF_EVENT_STATE_ERROR' val=-2
>          'PERF_EVENT_STATE_OFF' val=-1
>          'PERF_EVENT_STATE_INACTIVE' val=0
>          'PERF_EVENT_STATE_ACTIVE' val=1

> We are not currently using CONFIG_DEBUG_INFO_BTF_MODULES and
> don't plan to use module BTF, so it's preferable to be able
> to explicits disable it in the kernel config. Unfortunately,
> because that kconfig option doesn't have a name, it's not
> possible to flip it independently from CONFIG_DEBUG_INFO_BTF.
> Let's add a name to make sure module BTF is user-controllable.

[..]

> (Not sure, but maybe the right fix is to also have a stable patch
>   to relax that "Invalid btf_info kind_flag" check?)

Answering to myself, looks like we do need the following for
non-enum64-compatible older/stable kernels:

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 3cfba41a0829..928f4955090a 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -3301,11 +3301,6 @@ static s32 btf_enum_check_meta(struct  
btf_verifier_env *env,
  		return -EINVAL;
  	}

-	if (btf_type_kflag(t)) {
-		btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
-		return -EINVAL;
-	}
-
  	if (t->size > 8 || !is_power_of_2(t->size)) {
  		btf_verifier_log_type(env, t, "Unexpected size");
  		return -EINVAL;

Anything I'm missing? Feels like any pre-6089fb325cf7 ("bpf: Add btf
enum64 support") kernel will have an issue with a recent clang
that puts sign into kflag?


> Fixes: 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled  
> and pahole supports it")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>   lib/Kconfig.debug | 1 +
>   1 file changed, 1 insertion(+)

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index c77fe36bb3d8..6336a697c9f5 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -326,6 +326,7 @@ config PAHOLE_HAS_SPLIT_BTF
>   	def_bool $(success, test `$(PAHOLE) --version | sed  
> -E 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "119")

>   config DEBUG_INFO_BTF_MODULES
> +	bool "Generate BTF module typeinfo"
>   	def_bool y
>   	depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF
>   	help
> --
> 2.38.0.rc1.362.ged0d419d3c-goog