bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest
@ 2020-03-10  7:32 Yoshiki Komachi
  2020-03-10  7:32 ` [PATCH bpf 1/2] bpf/btf: Fix BTF verification of enum members in struct/union Yoshiki Komachi
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Yoshiki Komachi @ 2020-03-10  7:32 UTC (permalink / raw)
  To: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko
  Cc: Yoshiki Komachi, netdev, bpf

btf_enum_check_member() checked if the size of "enum" as a struct
member exceeded struct_size or not. Then, the function compared it
with the size of "int". Although the size of "enum" is 4-byte by
default (i.e., equivalent to "int"), the packing feature enables
us to reduce it, as illustrated by the following example:

struct A {
        char m;
        enum { E0, E1 } __attribute__((packed)) n;
};

With such a setup above, the bpf loader gave an error attempting
to load it:

------------------------------------------------------------------
...

[3] ENUM (anon) size=1 vlen=2
        E0 val=0
        E1 val=1
[4] STRUCT A size=2 vlen=2
        m type_id=2 bits_offset=0
        n type_id=3 bits_offset=8

[4] STRUCT A size=2 vlen=2
        n type_id=3 bits_offset=8 Member exceeds struct_size

libbpf: Error loading .BTF into kernel: -22.

------------------------------------------------------------------

The related issue was previously fixed by the commit 9eea98497951 ("bpf:
fix BTF verification of enums"). On the other hand, this series fixes
this issue as well, and adds a selftest program for it.

Changes in v2:
- change an example in commit message based on Andrii's review
- add a selftest program for packed "enum" type members in struct/union

Yoshiki Komachi (2):
  bpf/btf: Fix BTF verification of enum members in struct/union
  selftests/bpf: Add test for the packed enum member in struct/union

 kernel/bpf/btf.c                       |  2 +-
 tools/testing/selftests/bpf/test_btf.c | 42 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

-- 
1.8.3.1


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

* [PATCH bpf 1/2] bpf/btf: Fix BTF verification of enum members in struct/union
  2020-03-10  7:32 [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Yoshiki Komachi
@ 2020-03-10  7:32 ` Yoshiki Komachi
  2020-03-10  7:32 ` [PATCH bpf 2/2] selftests/bpf: Add test for the packed enum member " Yoshiki Komachi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Yoshiki Komachi @ 2020-03-10  7:32 UTC (permalink / raw)
  To: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko
  Cc: Yoshiki Komachi, netdev, bpf

btf_enum_check_member() was currently sure to recognize the size of
"enum" type members in struct/union as the size of "int" even if
its size was packed.

This patch fixes BTF enum verification to use the correct size
of member in BPF programs.

Fixes: 179cde8cef7e ("bpf: btf: Check members of struct/union")
Signed-off-by: Yoshiki Komachi <komachi.yoshiki@gmail.com>
---
 kernel/bpf/btf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 7871400..32ab922 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -2418,7 +2418,7 @@ static int btf_enum_check_member(struct btf_verifier_env *env,
 
 	struct_size = struct_type->size;
 	bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off);
-	if (struct_size - bytes_offset < sizeof(int)) {
+	if (struct_size - bytes_offset < member_type->size) {
 		btf_verifier_log_member(env, struct_type, member,
 					"Member exceeds struct_size");
 		return -EINVAL;
-- 
1.8.3.1


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

* [PATCH bpf 2/2] selftests/bpf: Add test for the packed enum member in struct/union
  2020-03-10  7:32 [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Yoshiki Komachi
  2020-03-10  7:32 ` [PATCH bpf 1/2] bpf/btf: Fix BTF verification of enum members in struct/union Yoshiki Komachi
@ 2020-03-10  7:32 ` Yoshiki Komachi
  2020-03-10 15:39 ` [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Andrii Nakryiko
  2020-03-10 16:46 ` Martin KaFai Lau
  3 siblings, 0 replies; 6+ messages in thread
From: Yoshiki Komachi @ 2020-03-10  7:32 UTC (permalink / raw)
  To: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko
  Cc: Yoshiki Komachi, netdev, bpf

Add a simple test to the existing selftest program in order to make
sure that a packed enum member in struct unexceeds the struct_size.

Signed-off-by: Yoshiki Komachi <komachi.yoshiki@gmail.com>
---
 tools/testing/selftests/bpf/test_btf.c | 42 ++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index 93040ca..8da77cd 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -1062,6 +1062,48 @@ struct btf_raw_test {
 	.err_str = "Member exceeds struct_size",
 },
 
+/* Test member unexceeds the size of struct
+ *
+ * enum E {
+ *     E0,
+ *     E1,
+ * };
+ *
+ * struct A {
+ *     char m;
+ *     enum E __attribute__((packed)) n;
+ * };
+ */
+{
+	.descr = "size check test #5",
+	.raw_types = {
+		/* int */			/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, sizeof(int)),
+		/* char */			/* [2] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 8, 1),
+		/* enum E { */			/* [3] */
+		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_ENUM, 0, 2), 1),
+		BTF_ENUM_ENC(NAME_TBD, 0),
+		BTF_ENUM_ENC(NAME_TBD, 1),
+		/* } */
+		/* struct A { */		/* [4] */
+		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 2), 2),
+		BTF_MEMBER_ENC(NAME_TBD, 2, 0),	/* char m; */
+		BTF_MEMBER_ENC(NAME_TBD, 3, 8),/* enum E __attribute__((packed)) n; */
+		/* } */
+		BTF_END_RAW,
+	},
+	.str_sec = "\0E\0E0\0E1\0A\0m\0n",
+	.str_sec_size = sizeof("\0E\0E0\0E1\0A\0m\0n"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "size_check5_map",
+	.key_size = sizeof(int),
+	.value_size = 2,
+	.key_type_id = 1,
+	.value_type_id = 4,
+	.max_entries = 4,
+},
+
 /* typedef const void * const_void_ptr;
  * struct A {
  *	const_void_ptr m;
-- 
1.8.3.1


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

* Re: [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest
  2020-03-10  7:32 [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Yoshiki Komachi
  2020-03-10  7:32 ` [PATCH bpf 1/2] bpf/btf: Fix BTF verification of enum members in struct/union Yoshiki Komachi
  2020-03-10  7:32 ` [PATCH bpf 2/2] selftests/bpf: Add test for the packed enum member " Yoshiki Komachi
@ 2020-03-10 15:39 ` Andrii Nakryiko
  2020-03-10 16:46 ` Martin KaFai Lau
  3 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2020-03-10 15:39 UTC (permalink / raw)
  To: Yoshiki Komachi
  Cc: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Networking, bpf

On Tue, Mar 10, 2020 at 12:32 AM Yoshiki Komachi
<komachi.yoshiki@gmail.com> wrote:
>
> btf_enum_check_member() checked if the size of "enum" as a struct
> member exceeded struct_size or not. Then, the function compared it
> with the size of "int". Although the size of "enum" is 4-byte by
> default (i.e., equivalent to "int"), the packing feature enables
> us to reduce it, as illustrated by the following example:
>
> struct A {
>         char m;
>         enum { E0, E1 } __attribute__((packed)) n;
> };
>
> With such a setup above, the bpf loader gave an error attempting
> to load it:
>
> ------------------------------------------------------------------
> ...
>
> [3] ENUM (anon) size=1 vlen=2
>         E0 val=0
>         E1 val=1
> [4] STRUCT A size=2 vlen=2
>         m type_id=2 bits_offset=0
>         n type_id=3 bits_offset=8
>
> [4] STRUCT A size=2 vlen=2
>         n type_id=3 bits_offset=8 Member exceeds struct_size
>
> libbpf: Error loading .BTF into kernel: -22.
>
> ------------------------------------------------------------------
>
> The related issue was previously fixed by the commit 9eea98497951 ("bpf:
> fix BTF verification of enums"). On the other hand, this series fixes
> this issue as well, and adds a selftest program for it.
>
> Changes in v2:
> - change an example in commit message based on Andrii's review
> - add a selftest program for packed "enum" type members in struct/union
>
> Yoshiki Komachi (2):
>   bpf/btf: Fix BTF verification of enum members in struct/union
>   selftests/bpf: Add test for the packed enum member in struct/union
>
>  kernel/bpf/btf.c                       |  2 +-
>  tools/testing/selftests/bpf/test_btf.c | 42 ++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 1 deletion(-)
>
> --
> 1.8.3.1
>

You should have updated patch prefix for patch #1 and #2 to [PATCH v2
bpf] as well, please do it next time,

For the series:

Acked-by: Andrii Nakryiko <andriin@fb.com>

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

* Re: [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest
  2020-03-10  7:32 [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Yoshiki Komachi
                   ` (2 preceding siblings ...)
  2020-03-10 15:39 ` [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Andrii Nakryiko
@ 2020-03-10 16:46 ` Martin KaFai Lau
  2020-03-10 17:26   ` Alexei Starovoitov
  3 siblings, 1 reply; 6+ messages in thread
From: Martin KaFai Lau @ 2020-03-10 16:46 UTC (permalink / raw)
  To: Yoshiki Komachi
  Cc: David S. Miller, Alexei Starovoitov, Daniel Borkmann, Song Liu,
	Yonghong Song, Andrii Nakryiko, netdev, bpf

On Tue, Mar 10, 2020 at 04:32:28PM +0900, Yoshiki Komachi wrote:
> btf_enum_check_member() checked if the size of "enum" as a struct
> member exceeded struct_size or not. Then, the function compared it
> with the size of "int". Although the size of "enum" is 4-byte by
> default (i.e., equivalent to "int"), the packing feature enables
> us to reduce it, as illustrated by the following example:
> 
> struct A {
>         char m;
>         enum { E0, E1 } __attribute__((packed)) n;
> };
> 
> With such a setup above, the bpf loader gave an error attempting
> to load it:
> 
> ------------------------------------------------------------------
> ...
> 
> [3] ENUM (anon) size=1 vlen=2
>         E0 val=0
>         E1 val=1
> [4] STRUCT A size=2 vlen=2
>         m type_id=2 bits_offset=0
>         n type_id=3 bits_offset=8
> 
> [4] STRUCT A size=2 vlen=2
>         n type_id=3 bits_offset=8 Member exceeds struct_size
> 
> libbpf: Error loading .BTF into kernel: -22.
> 
> ------------------------------------------------------------------
> 
> The related issue was previously fixed by the commit 9eea98497951 ("bpf:
> fix BTF verification of enums"). On the other hand, this series fixes
> this issue as well, and adds a selftest program for it.
> 
> Changes in v2:
> - change an example in commit message based on Andrii's review
> - add a selftest program for packed "enum" type members in struct/union
Acked-by: Martin KaFai Lau <kafai@fb.com>

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

* Re: [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest
  2020-03-10 16:46 ` Martin KaFai Lau
@ 2020-03-10 17:26   ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2020-03-10 17:26 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Yoshiki Komachi, David S. Miller, Alexei Starovoitov,
	Daniel Borkmann, Song Liu, Yonghong Song, Andrii Nakryiko,
	Network Development, bpf

On Tue, Mar 10, 2020 at 9:46 AM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Tue, Mar 10, 2020 at 04:32:28PM +0900, Yoshiki Komachi wrote:
> > btf_enum_check_member() checked if the size of "enum" as a struct
> > member exceeded struct_size or not. Then, the function compared it
> > with the size of "int". Although the size of "enum" is 4-byte by
> > default (i.e., equivalent to "int"), the packing feature enables
> > us to reduce it, as illustrated by the following example:
> >
> > struct A {
> >         char m;
> >         enum { E0, E1 } __attribute__((packed)) n;
> > };
> >
> > With such a setup above, the bpf loader gave an error attempting
> > to load it:
> >
> > ------------------------------------------------------------------
> > ...
> >
> > [3] ENUM (anon) size=1 vlen=2
> >         E0 val=0
> >         E1 val=1
> > [4] STRUCT A size=2 vlen=2
> >         m type_id=2 bits_offset=0
> >         n type_id=3 bits_offset=8
> >
> > [4] STRUCT A size=2 vlen=2
> >         n type_id=3 bits_offset=8 Member exceeds struct_size
> >
> > libbpf: Error loading .BTF into kernel: -22.
> >
> > ------------------------------------------------------------------
> >
> > The related issue was previously fixed by the commit 9eea98497951 ("bpf:
> > fix BTF verification of enums"). On the other hand, this series fixes
> > this issue as well, and adds a selftest program for it.
> >
> > Changes in v2:
> > - change an example in commit message based on Andrii's review
> > - add a selftest program for packed "enum" type members in struct/union
> Acked-by: Martin KaFai Lau <kafai@fb.com>

Applied to bpf tree. Thanks

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

end of thread, other threads:[~2020-03-10 17:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-10  7:32 [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Yoshiki Komachi
2020-03-10  7:32 ` [PATCH bpf 1/2] bpf/btf: Fix BTF verification of enum members in struct/union Yoshiki Komachi
2020-03-10  7:32 ` [PATCH bpf 2/2] selftests/bpf: Add test for the packed enum member " Yoshiki Komachi
2020-03-10 15:39 ` [PATCH v2 bpf 0/2] Fix BTF verification of enum members with a selftest Andrii Nakryiko
2020-03-10 16:46 ` Martin KaFai Lau
2020-03-10 17:26   ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).