All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 1/2] bpf: Silence a warning in btf_type_id_size()
@ 2023-05-27 22:31 Yonghong Song
  2023-05-27 22:31 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test where map key_type_id with decl_tag type Yonghong Song
  0 siblings, 1 reply; 4+ messages in thread
From: Yonghong Song @ 2023-05-27 22:31 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau, syzbot+958967f249155967d42a

syzbot reported a warning in [1] with the following stacktrace:
  WARNING: CPU: 0 PID: 5005 at kernel/bpf/btf.c:1988 btf_type_id_size+0x2d9/0x9d0 kernel/bpf/btf.c:1988
  ...
  RIP: 0010:btf_type_id_size+0x2d9/0x9d0 kernel/bpf/btf.c:1988
  ...
  Call Trace:
   <TASK>
   map_check_btf kernel/bpf/syscall.c:1024 [inline]
   map_create+0x1157/0x1860 kernel/bpf/syscall.c:1198
   __sys_bpf+0x127f/0x5420 kernel/bpf/syscall.c:5040
   __do_sys_bpf kernel/bpf/syscall.c:5162 [inline]
   __se_sys_bpf kernel/bpf/syscall.c:5160 [inline]
   __x64_sys_bpf+0x79/0xc0 kernel/bpf/syscall.c:5160
   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
   do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
   entry_SYSCALL_64_after_hwframe+0x63/0xcd

With the following btf
  [1] DECL_TAG 'a' type_id=4 component_idx=-1
  [2] PTR '(anon)' type_id=0
  [3] TYPE_TAG 'a' type_id=2
  [4] VAR 'a' type_id=3, linkage=static
and when the bpf_attr.btf_key_type_id = 1 (DECL_TAG),
the following WARN_ON_ONCE in btf_type_id_size() is triggered:
  if (WARN_ON_ONCE(!btf_type_is_modifier(size_type) &&
                   !btf_type_is_var(size_type)))
          return NULL;

Note that 'return NULL' is the correct behavior as we don't want
a DECL_TAG type to be used as a btf_{key,value}_type_id even
for the case like 'DECL_TAG -> STRUCT'. So there
is no correctness issue here, we just want to silence warning.

To silence the warning, I added DECL_TAG as one of kinds in
btf_type_nosize() which will cause btf_type_id_size() returning
NULL earlier without the warning.

  [1] https://lore.kernel.org/bpf/000000000000e0df8d05fc75ba86@google.com/

Reported-by: syzbot+958967f249155967d42a@syzkaller.appspotmail.com
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 kernel/bpf/btf.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 947f0b83bfad..bd2cac057928 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -492,25 +492,26 @@ static bool btf_type_is_fwd(const struct btf_type *t)
 	return BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
 }
 
-static bool btf_type_nosize(const struct btf_type *t)
+static bool btf_type_is_datasec(const struct btf_type *t)
 {
-	return btf_type_is_void(t) || btf_type_is_fwd(t) ||
-	       btf_type_is_func(t) || btf_type_is_func_proto(t);
+	return BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC;
 }
 
-static bool btf_type_nosize_or_null(const struct btf_type *t)
+static bool btf_type_is_decl_tag(const struct btf_type *t)
 {
-	return !t || btf_type_nosize(t);
+	return BTF_INFO_KIND(t->info) == BTF_KIND_DECL_TAG;
 }
 
-static bool btf_type_is_datasec(const struct btf_type *t)
+static bool btf_type_nosize(const struct btf_type *t)
 {
-	return BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC;
+	return btf_type_is_void(t) || btf_type_is_fwd(t) ||
+	       btf_type_is_func(t) || btf_type_is_func_proto(t) ||
+	       btf_type_is_decl_tag(t);
 }
 
-static bool btf_type_is_decl_tag(const struct btf_type *t)
+static bool btf_type_nosize_or_null(const struct btf_type *t)
 {
-	return BTF_INFO_KIND(t->info) == BTF_KIND_DECL_TAG;
+	return !t || btf_type_nosize(t);
 }
 
 static bool btf_type_is_decl_tag_target(const struct btf_type *t)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: Add a test where map key_type_id with decl_tag type
  2023-05-27 22:31 [PATCH bpf-next 1/2] bpf: Silence a warning in btf_type_id_size() Yonghong Song
@ 2023-05-27 22:31 ` Yonghong Song
  2023-05-29 15:17   ` Eduard Zingerman
  0 siblings, 1 reply; 4+ messages in thread
From: Yonghong Song @ 2023-05-27 22:31 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau

Add a selftest where map creation key type_id is a decl_tag
pointing to a struct. Without previous patch, a kernel warning will
appear similar to the one in the previous patch. With the previous
patch, the kernel warning is silenced.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/prog_tests/btf.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 210d643fda6c..69521e1dc330 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3990,6 +3990,26 @@ static struct btf_raw_test raw_tests[] = {
 	.btf_load_err = true,
 	.err_str = "Invalid arg#1",
 },
+{
+	.descr = "decl_tag test #18, struct member, decl_tag as the value type",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_STRUCT_ENC(0, 2, 8),			/* [2] */
+		BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+		BTF_MEMBER_ENC(NAME_TBD, 1, 32),
+		BTF_DECL_TAG_ENC(NAME_TBD, 2, -1),		/* [3] */
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0m1\0m2\0tag"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "tag_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = 8,
+	.key_type_id = 1,
+	.value_type_id = 3,
+	.max_entries = 1,
+	.map_create_err = true,
+},
 {
 	.descr = "type_tag test #1",
 	.raw_types = {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next 2/2] selftests/bpf: Add a test where map key_type_id with decl_tag type
  2023-05-27 22:31 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test where map key_type_id with decl_tag type Yonghong Song
@ 2023-05-29 15:17   ` Eduard Zingerman
  2023-05-30 17:48     ` Yonghong Song
  0 siblings, 1 reply; 4+ messages in thread
From: Eduard Zingerman @ 2023-05-29 15:17 UTC (permalink / raw)
  To: Yonghong Song, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau

On Sat, 2023-05-27 at 15:31 -0700, Yonghong Song wrote:
> Add a selftest where map creation key type_id is a decl_tag
> pointing to a struct. Without previous patch, a kernel warning will
> appear similar to the one in the previous patch. With the previous
> patch, the kernel warning is silenced.

Looks good to me with a nitpick:
commit message says "map creation key type_id is a decl_tag",
but test case uses ".key_type_id = 1" which is INT
and ".value_type_id = 3" which is DECL_TAG.

syscall.c:map_check_btf.c applies the same check both for key and value,
maybe make two tests?

> 
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/btf.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 210d643fda6c..69521e1dc330 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -3990,6 +3990,26 @@ static struct btf_raw_test raw_tests[] = {
>  	.btf_load_err = true,
>  	.err_str = "Invalid arg#1",
>  },
> +{
> +	.descr = "decl_tag test #18, struct member, decl_tag as the value type",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_STRUCT_ENC(0, 2, 8),			/* [2] */
> +		BTF_MEMBER_ENC(NAME_TBD, 1, 0),
> +		BTF_MEMBER_ENC(NAME_TBD, 1, 32),
> +		BTF_DECL_TAG_ENC(NAME_TBD, 2, -1),		/* [3] */
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0m1\0m2\0tag"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "tag_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 8,
> +	.key_type_id = 1,
> +	.value_type_id = 3,
> +	.max_entries = 1,
> +	.map_create_err = true,
> +},
>  {
>  	.descr = "type_tag test #1",
>  	.raw_types = {


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next 2/2] selftests/bpf: Add a test where map key_type_id with decl_tag type
  2023-05-29 15:17   ` Eduard Zingerman
@ 2023-05-30 17:48     ` Yonghong Song
  0 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2023-05-30 17:48 UTC (permalink / raw)
  To: Eduard Zingerman, Yonghong Song, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau



On 5/29/23 8:17 AM, Eduard Zingerman wrote:
> On Sat, 2023-05-27 at 15:31 -0700, Yonghong Song wrote:
>> Add a selftest where map creation key type_id is a decl_tag
>> pointing to a struct. Without previous patch, a kernel warning will
>> appear similar to the one in the previous patch. With the previous
>> patch, the kernel warning is silenced.
> 
> Looks good to me with a nitpick:
> commit message says "map creation key type_id is a decl_tag",
> but test case uses ".key_type_id = 1" which is INT
> and ".value_type_id = 3" which is DECL_TAG.
> 
> syscall.c:map_check_btf.c applies the same check both for key and value,
> maybe make two tests?

Sounds good. Will fix the commit message and add one more test so both
key and value are covered.

> 
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/testing/selftests/bpf/prog_tests/btf.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
>> index 210d643fda6c..69521e1dc330 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
>> @@ -3990,6 +3990,26 @@ static struct btf_raw_test raw_tests[] = {
>>   	.btf_load_err = true,
>>   	.err_str = "Invalid arg#1",
>>   },
>> +{
>> +	.descr = "decl_tag test #18, struct member, decl_tag as the value type",
>> +	.raw_types = {
>> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
>> +		BTF_STRUCT_ENC(0, 2, 8),			/* [2] */
>> +		BTF_MEMBER_ENC(NAME_TBD, 1, 0),
>> +		BTF_MEMBER_ENC(NAME_TBD, 1, 32),
>> +		BTF_DECL_TAG_ENC(NAME_TBD, 2, -1),		/* [3] */
>> +		BTF_END_RAW,
>> +	},
>> +	BTF_STR_SEC("\0m1\0m2\0tag"),
>> +	.map_type = BPF_MAP_TYPE_ARRAY,
>> +	.map_name = "tag_type_check_btf",
>> +	.key_size = sizeof(int),
>> +	.value_size = 8,
>> +	.key_type_id = 1,
>> +	.value_type_id = 3,
>> +	.max_entries = 1,
>> +	.map_create_err = true,
>> +},
>>   {
>>   	.descr = "type_tag test #1",
>>   	.raw_types = {
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-05-30 17:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-27 22:31 [PATCH bpf-next 1/2] bpf: Silence a warning in btf_type_id_size() Yonghong Song
2023-05-27 22:31 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test where map key_type_id with decl_tag type Yonghong Song
2023-05-29 15:17   ` Eduard Zingerman
2023-05-30 17:48     ` Yonghong Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.