[PATCH bpf-next 1/2] selftests/bpf: Add reproducer for decl_tag in func_proto argument

[PATCH bpf-next 1/2] selftests/bpf: Add reproducer for decl_tag in func_proto argument

[Stanislav Fomichev]

It should trigger a WARN_ON_ONCE in btf_type_id_size.

    RIP: 0010:btf_type_id_size+0x8bd/0x940 kernel/bpf/btf.c:1952
    btf_func_proto_check kernel/bpf/btf.c:4506 [inline]
    btf_check_all_types kernel/bpf/btf.c:4734 [inline]
    btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763
    btf_parse kernel/bpf/btf.c:5042 [inline]
    btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709
    bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342
    __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034
    __do_sys_bpf kernel/bpf/syscall.c:5093 [inline]
    __se_sys_bpf kernel/bpf/syscall.c:5091 [inline]
    __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091
    do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/testing/selftests/bpf/prog_tests/btf.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 95a2b80f0d17..de1b5b9eb93a 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3948,6 +3948,20 @@ static struct btf_raw_test raw_tests[] = {
 	.btf_load_err = true,
 	.err_str = "Invalid return type",
 },
+{
+	.descr = "decl_tag test #17, func proto, argument",
+	.raw_types = {
+		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_DECL_TAG, 0, 0), 4), (-1),	/* [1] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 0), /* [2] */
+		BTF_FUNC_PROTO_ENC(0, 1),			/* [3] */
+			BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
+		BTF_VAR_ENC(NAME_TBD, 2, 0),			/* [4] */
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0local\0tag1\0var"),
+	.btf_load_err = true,
+	.err_str = "Invalid arg#1",
+},
 {
 	.descr = "type_tag test #1",
 	.raw_types = {
-- 
2.38.1.584.g0f3c55d4c2-goog

[PATCH bpf-next 2/2] bpf: prevent decl_tag from being referenced in func_proto arg

[Stanislav Fomichev]

Syzkaller managed to hit anoher decl_tag issue:

 btf_func_proto_check kernel/bpf/btf.c:4506 [inline]
 btf_check_all_types kernel/bpf/btf.c:4734 [inline]
 btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763
 btf_parse kernel/bpf/btf.c:5042 [inline]
 btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709
 bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342
 __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034
 __do_sys_bpf kernel/bpf/syscall.c:5093 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:5091 [inline]
 __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091
 do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48

This seems similar to commit ea68376c8bed ("bpf: prevent decl_tag from
being referenced in func_proto") but for the argument.

Reported-by: syzbot+8dd0551dda6020944c5d@syzkaller.appspotmail.com
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 kernel/bpf/btf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 1a59cc7ad730..cb43cb842e16 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4792,6 +4792,11 @@ static int btf_func_proto_check(struct btf_verifier_env *env,
 			break;
 		}
 
+		if (btf_type_is_resolve_source_only(arg_type)) {
+			btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1);
+			return -EINVAL;
+		}
+
 		if (args[i].name_off &&
 		    (!btf_name_offset_valid(btf, args[i].name_off) ||
 		     !btf_name_valid_identifier(btf, args[i].name_off))) {
-- 
2.38.1.584.g0f3c55d4c2-goog

Re: [PATCH bpf-next 2/2] bpf: prevent decl_tag from being referenced in func_proto arg

[Yonghong Song]

On 11/22/22 7:54 PM, Stanislav Fomichev wrote:
> Syzkaller managed to hit anoher decl_tag issue:
> 
>   btf_func_proto_check kernel/bpf/btf.c:4506 [inline]
>   btf_check_all_types kernel/bpf/btf.c:4734 [inline]
>   btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763
>   btf_parse kernel/bpf/btf.c:5042 [inline]
>   btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709
>   bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342
>   __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034
>   __do_sys_bpf kernel/bpf/syscall.c:5093 [inline]
>   __se_sys_bpf kernel/bpf/syscall.c:5091 [inline]
>   __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091
>   do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48
> 
> This seems similar to commit ea68376c8bed ("bpf: prevent decl_tag from
> being referenced in func_proto") but for the argument.
> 
> Reported-by: syzbot+8dd0551dda6020944c5d@syzkaller.appspotmail.com
> Signed-off-by: Stanislav Fomichev <sdf@google.com>

Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   kernel/bpf/btf.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 1a59cc7ad730..cb43cb842e16 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4792,6 +4792,11 @@ static int btf_func_proto_check(struct btf_verifier_env *env,
>   			break;
>   		}
>   
> +		if (btf_type_is_resolve_source_only(arg_type)) {
> +			btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1);
> +			return -EINVAL;
> +		}
> +
>   		if (args[i].name_off &&
>   		    (!btf_name_offset_valid(btf, args[i].name_off) ||
>   		     !btf_name_valid_identifier(btf, args[i].name_off))) {

Re: [PATCH bpf-next 2/2] bpf: prevent decl_tag from being referenced in func_proto arg

[Daniel Borkmann]

On 11/23/22 4:54 AM, Stanislav Fomichev wrote:
> Syzkaller managed to hit anoher decl_tag issue:
> 
>   btf_func_proto_check kernel/bpf/btf.c:4506 [inline]
>   btf_check_all_types kernel/bpf/btf.c:4734 [inline]
>   btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763
>   btf_parse kernel/bpf/btf.c:5042 [inline]
>   btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709
>   bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342
>   __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034
>   __do_sys_bpf kernel/bpf/syscall.c:5093 [inline]
>   __se_sys_bpf kernel/bpf/syscall.c:5091 [inline]
>   __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091
>   do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48
> 
> This seems similar to commit ea68376c8bed ("bpf: prevent decl_tag from
> being referenced in func_proto") but for the argument.
> 
> Reported-by: syzbot+8dd0551dda6020944c5d@syzkaller.appspotmail.com
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>   kernel/bpf/btf.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 1a59cc7ad730..cb43cb842e16 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4792,6 +4792,11 @@ static int btf_func_proto_check(struct btf_verifier_env *env,
>   			break;
>   		}
>   
> +		if (btf_type_is_resolve_source_only(arg_type)) {
> +			btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1);
> +			return -EINVAL;
> +		}
> +

Applied, could you do a small follow-up cleanup: most of the error cases in the loop in
btf_func_proto_check() bail out with err = -EINVAL + break and the above now deviates
from that, but rightfully so given there's no good reason as we just return err anyway.
Would be good to make this consistent with return -EINVAL / return err also for the other
cases.

>   		if (args[i].name_off &&
>   		    (!btf_name_offset_valid(btf, args[i].name_off) ||
>   		     !btf_name_valid_identifier(btf, args[i].name_off))) {
> 

Thanks,
Daniel

Re: [PATCH bpf-next 1/2] selftests/bpf: Add reproducer for decl_tag in func_proto argument

[Yonghong Song]

On 11/22/22 7:54 PM, Stanislav Fomichev wrote:
> It should trigger a WARN_ON_ONCE in btf_type_id_size.
> 
>      RIP: 0010:btf_type_id_size+0x8bd/0x940 kernel/bpf/btf.c:1952
>      btf_func_proto_check kernel/bpf/btf.c:4506 [inline]
>      btf_check_all_types kernel/bpf/btf.c:4734 [inline]
>      btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763
>      btf_parse kernel/bpf/btf.c:5042 [inline]
>      btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709
>      bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342
>      __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034
>      __do_sys_bpf kernel/bpf/syscall.c:5093 [inline]
>      __se_sys_bpf kernel/bpf/syscall.c:5091 [inline]
>      __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091
>      do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48
> 
> Signed-off-by: Stanislav Fomichev <sdf@google.com>

Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   tools/testing/selftests/bpf/prog_tests/btf.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 95a2b80f0d17..de1b5b9eb93a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -3948,6 +3948,20 @@ static struct btf_raw_test raw_tests[] = {
>   	.btf_load_err = true,
>   	.err_str = "Invalid return type",
>   },
> +{
> +	.descr = "decl_tag test #17, func proto, argument",
> +	.raw_types = {
> +		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_DECL_TAG, 0, 0), 4), (-1),	/* [1] */
> +		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 0), /* [2] */
> +		BTF_FUNC_PROTO_ENC(0, 1),			/* [3] */
> +			BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
> +		BTF_VAR_ENC(NAME_TBD, 2, 0),			/* [4] */
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0local\0tag1\0var"),
> +	.btf_load_err = true,
> +	.err_str = "Invalid arg#1",
> +},
>   {
>   	.descr = "type_tag test #1",
>   	.raw_types = {

Re: [PATCH bpf-next 1/2] selftests/bpf: Add reproducer for decl_tag in func_proto argument

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Tue, 22 Nov 2022 19:54:21 -0800 you wrote:
> It should trigger a WARN_ON_ONCE in btf_type_id_size.
> 
>     RIP: 0010:btf_type_id_size+0x8bd/0x940 kernel/bpf/btf.c:1952
>     btf_func_proto_check kernel/bpf/btf.c:4506 [inline]
>     btf_check_all_types kernel/bpf/btf.c:4734 [inline]
>     btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763
>     btf_parse kernel/bpf/btf.c:5042 [inline]
>     btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709
>     bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342
>     __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034
>     __do_sys_bpf kernel/bpf/syscall.c:5093 [inline]
>     __se_sys_bpf kernel/bpf/syscall.c:5091 [inline]
>     __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091
>     do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] selftests/bpf: Add reproducer for decl_tag in func_proto argument
    https://git.kernel.org/bpf/bpf-next/c/8e898aaa733e
  - [bpf-next,2/2] bpf: prevent decl_tag from being referenced in func_proto arg
    https://git.kernel.org/bpf/bpf-next/c/f17472d45996

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html