[PATCH bpf-next v4 0/3] bpf, btf: Add DEBUG_INFO_BTF checks for __register_bpf_struct_ops

[PATCH bpf-next v4 0/3] bpf, btf: Add DEBUG_INFO_BTF checks for __register_bpf_struct_ops

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v4:
 - add a new patch to fix error checks for btf_get_module_btf.
 - rename the helper to check_btf_kconfigs.

v3:
 - fix this build error:
kernel/bpf/btf.c:7750:11: error: incomplete definition of type 'struct module'

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202402040934.Fph0XeEo-lkp@intel.com/

v2:
 - add register_check_missing_btf helper as Jiri suggested.

Geliang Tang (3):
  bpf, btf: Fix error checks for btf_get_module_btf
  bpf, btf: Add check_btf_kconfigs helper
  bpf, btf: Check btf for register_bpf_struct_ops

 kernel/bpf/btf.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

-- 
2.40.1

[PATCH bpf-next v4 1/3] bpf, btf: Fix error checks for btf_get_module_btf

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

To let the modules loaded, commit 3de4d22cc9ac ("bpf, btf: Warn but
return no error for NULL btf from __register_btf_kfunc_id_set()")
changes the return value of __register_btf_kfunc_id_set() from -ENOENT
to 0 when btf is NULL.

A better way to do this is to enable CONFIG_MODULE_ALLOW_BTF_MISMATCH.

An error code -ENOENT should indeed be returned when kernel module BTF
mismatch detected except CONFIG_MODULE_ALLOW_BTF_MISMATCH is enabled in
__register_btf_kfunc_id_set().

The same in register_btf_id_dtor_kfuncs(), give the modules a chance
to be loaded when CONFIG_MODULE_ALLOW_BTF_MISMATCH is enabled.

Fixes: 3de4d22cc9ac ("bpf, btf: Warn but return no error for NULL btf from __register_btf_kfunc_id_set()")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 kernel/bpf/btf.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index f7725cb6e564..203391e61d93 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8103,8 +8103,11 @@ static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
 			pr_err("missing vmlinux BTF, cannot register kfuncs\n");
 			return -ENOENT;
 		}
-		if (kset->owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
+		if (kset->owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) &&
+		    !IS_ENABLED(CONFIG_MODULE_ALLOW_BTF_MISMATCH)) {
 			pr_warn("missing module BTF, cannot register kfuncs\n");
+			return -ENOENT;
+		}
 		return 0;
 	}
 	if (IS_ERR(btf))
@@ -8219,7 +8222,8 @@ int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_c
 			pr_err("missing vmlinux BTF, cannot register dtor kfuncs\n");
 			return -ENOENT;
 		}
-		if (owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES)) {
+		if (owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) &&
+		    !IS_ENABLED(CONFIG_MODULE_ALLOW_BTF_MISMATCH)) {
 			pr_err("missing module BTF, cannot register dtor kfuncs\n");
 			return -ENOENT;
 		}
-- 
2.40.1

[PATCH bpf-next v4 2/3] bpf, btf: Add check_btf_kconfigs helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch extracts duplicate code on error path when btf_get_module_btf()
returns NULL from the functions __register_btf_kfunc_id_set() and
register_btf_id_dtor_kfuncs() into a new helper named check_btf_kconfigs()
to check CONFIG_DEBUG_INFO_BTF, CONFIG_DEBUG_INFO_BTF_MODULES and
CONFIG_MODULE_ALLOW_BTF_MISMATCH in it.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 kernel/bpf/btf.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 203391e61d93..eedbee04de89 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7738,6 +7738,20 @@ static struct btf *btf_get_module_btf(const struct module *module)
 	return btf;
 }
 
+static int check_btf_kconfigs(const struct module *module)
+{
+	if (!module && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) {
+		pr_err("missing vmlinux BTF, cannot register kfuncs\n");
+		return -ENOENT;
+	}
+	if (module && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) &&
+	    !IS_ENABLED(CONFIG_MODULE_ALLOW_BTF_MISMATCH)) {
+		pr_err("missing module BTF, cannot register kfuncs\n");
+		return -ENOENT;
+	}
+	return 0;
+}
+
 BPF_CALL_4(bpf_btf_find_by_name_kind, char *, name, int, name_sz, u32, kind, int, flags)
 {
 	struct btf *btf = NULL;
@@ -8098,18 +8112,8 @@ static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
 	int ret, i;
 
 	btf = btf_get_module_btf(kset->owner);
-	if (!btf) {
-		if (!kset->owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) {
-			pr_err("missing vmlinux BTF, cannot register kfuncs\n");
-			return -ENOENT;
-		}
-		if (kset->owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) &&
-		    !IS_ENABLED(CONFIG_MODULE_ALLOW_BTF_MISMATCH)) {
-			pr_warn("missing module BTF, cannot register kfuncs\n");
-			return -ENOENT;
-		}
-		return 0;
-	}
+	if (!btf)
+		return check_btf_kconfigs(kset->owner);
 	if (IS_ERR(btf))
 		return PTR_ERR(btf);
 
@@ -8217,18 +8221,8 @@ int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_c
 	int ret;
 
 	btf = btf_get_module_btf(owner);
-	if (!btf) {
-		if (!owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) {
-			pr_err("missing vmlinux BTF, cannot register dtor kfuncs\n");
-			return -ENOENT;
-		}
-		if (owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) &&
-		    !IS_ENABLED(CONFIG_MODULE_ALLOW_BTF_MISMATCH)) {
-			pr_err("missing module BTF, cannot register dtor kfuncs\n");
-			return -ENOENT;
-		}
-		return 0;
-	}
+	if (!btf)
+		return check_btf_kconfigs(owner);
 	if (IS_ERR(btf))
 		return PTR_ERR(btf);
 
-- 
2.40.1

[PATCH bpf-next v4 3/3] bpf, btf: Check btf for register_bpf_struct_ops

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Similar to the handling in the functions __register_btf_kfunc_id_set()
and register_btf_id_dtor_kfuncs(), this patch uses the newly added
helper check_btf_kconfigs() and IS_ERR() to check the return value of
btf_get_module_btf().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 kernel/bpf/btf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index eedbee04de89..9aae5a4bba24 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8885,7 +8885,9 @@ int __register_bpf_struct_ops(struct bpf_struct_ops *st_ops)
 
 	btf = btf_get_module_btf(st_ops->owner);
 	if (!btf)
-		return -EINVAL;
+		return check_btf_kconfigs(st_ops->owner);
+	if (IS_ERR(btf))
+		return PTR_ERR(btf);
 
 	log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
 	if (!log) {
-- 
2.40.1

Re: bpf, btf: Check btf for register_bpf_struct_ops: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): packetdrill_regressions 🔴:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7816265075

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/420e302e8846


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: bpf, btf: Check btf for register_bpf_struct_ops: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5151649740619776
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5151649740619776/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5834975782633472
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5834975782633472/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/420e302e8846


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH bpf-next v4 1/3] bpf, btf: Fix error checks for btf_get_module_btf

[Martin KaFai Lau]

On 2/7/24 6:07 AM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To let the modules loaded, commit 3de4d22cc9ac ("bpf, btf: Warn but
> return no error for NULL btf from __register_btf_kfunc_id_set()")
> changes the return value of __register_btf_kfunc_id_set() from -ENOENT
> to 0 when btf is NULL.
> 
> A better way to do this is to enable CONFIG_MODULE_ALLOW_BTF_MISMATCH.
> 
> An error code -ENOENT should indeed be returned when kernel module BTF
> mismatch detected except CONFIG_MODULE_ALLOW_BTF_MISMATCH is enabled in
> __register_btf_kfunc_id_set().
> 
> The same in register_btf_id_dtor_kfuncs(), give the modules a chance
> to be loaded when CONFIG_MODULE_ALLOW_BTF_MISMATCH is enabled.
> 
> Fixes: 3de4d22cc9ac ("bpf, btf: Warn but return no error for NULL btf from __register_btf_kfunc_id_set()")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>   kernel/bpf/btf.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index f7725cb6e564..203391e61d93 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8103,8 +8103,11 @@ static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
>   			pr_err("missing vmlinux BTF, cannot register kfuncs\n");
>   			return -ENOENT;
>   		}
> -		if (kset->owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))
> +		if (kset->owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) &&
> +		    !IS_ENABLED(CONFIG_MODULE_ALLOW_BTF_MISMATCH)) {

I am not sure this new test can be added here now. Otherwise, the existing 
kconfig that does not have CONFIG_MODULE_ALLOW_BTF_MISMATCH set will fail to 
load a module with btf section stripped out as described in commit 3de4d22cc9ac.

A module with stripped btf section does not mean BTF_MISMATCH also.

>   			pr_warn("missing module BTF, cannot register kfuncs\n");
> +			return -ENOENT;
> +		}
>   		return 0;
>   	}
>   	if (IS_ERR(btf))
> @@ -8219,7 +8222,8 @@ int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_c
>   			pr_err("missing vmlinux BTF, cannot register dtor kfuncs\n");
>   			return -ENOENT;
>   		}
> -		if (owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES)) {
> +		if (owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) &&
> +		    !IS_ENABLED(CONFIG_MODULE_ALLOW_BTF_MISMATCH)) {
>   			pr_err("missing module BTF, cannot register dtor kfuncs\n");
>   			return -ENOENT;
>   		}