* [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