* [PATCH bpf] bpf: btf: Fix BTF verification of the enum size in struct/union
@ 2020-02-28 2:02 Yoshiki Komachi
2020-02-29 0:23 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Yoshiki Komachi @ 2020-02-28 2:02 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 definitely
compared it with the size of "int" now. Therefore, errors could occur
when the size of the "enum" type was changed.
Although the size of "enum" is 4-byte by default, users can change it
as needed (e.g., the size of the following test variable is not 4-byte
but 1-byte). It can be used as a struct member as below:
enum test : char {
X,
Y,
Z,
};
struct {
char a;
enum test b;
char c;
} tmp;
With the setup above, when I tried to load BTF, the error was given
as below:
------------------------------------------------------------------
[58] STRUCT (anon) size=3 vlen=3
a type_id=55 bits_offset=0
b type_id=59 bits_offset=8
c type_id=55 bits_offset=16
[59] ENUM test size=1 vlen=3
X val=0
Y val=1
Z val=2
[58] STRUCT (anon) size=3 vlen=3
b type_id=59 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 patch fixes
my explained issue by using the correct size of "enum" declared 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] 3+ messages in thread
* Re: [PATCH bpf] bpf: btf: Fix BTF verification of the enum size in struct/union
2020-02-28 2:02 [PATCH bpf] bpf: btf: Fix BTF verification of the enum size in struct/union Yoshiki Komachi
@ 2020-02-29 0:23 ` Andrii Nakryiko
2020-03-02 6:57 ` Yoshiki Komachi
0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2020-02-29 0:23 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 Thu, Feb 27, 2020 at 6:07 PM 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 definitely
> compared it with the size of "int" now. Therefore, errors could occur
> when the size of the "enum" type was changed.
>
> Although the size of "enum" is 4-byte by default, users can change it
> as needed (e.g., the size of the following test variable is not 4-byte
> but 1-byte). It can be used as a struct member as below:
>
> enum test : char {
you can't specify that in pure C, but this will work for C:
struct {
enum { X, Y, Z } __attribute__((packed)) e;
} tmp;
Please add such a selftest, as part of fixing this bug. Thanks!
Otherwise logic looks good.
> X,
> Y,
> Z,
> };
>
> struct {
> char a;
> enum test b;
> char c;
> } tmp;
>
> With the setup above, when I tried to load BTF, the error was given
> as below:
>
> ------------------------------------------------------------------
>
> [58] STRUCT (anon) size=3 vlen=3
> a type_id=55 bits_offset=0
> b type_id=59 bits_offset=8
> c type_id=55 bits_offset=16
> [59] ENUM test size=1 vlen=3
> X val=0
> Y val=1
> Z val=2
>
> [58] STRUCT (anon) size=3 vlen=3
> b type_id=59 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 patch fixes
> my explained issue by using the correct size of "enum" declared 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 [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] bpf: btf: Fix BTF verification of the enum size in struct/union
2020-02-29 0:23 ` Andrii Nakryiko
@ 2020-03-02 6:57 ` Yoshiki Komachi
0 siblings, 0 replies; 3+ messages in thread
From: Yoshiki Komachi @ 2020-03-02 6:57 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Networking, bpf
2020年2月29日(土) 9:23 Andrii Nakryiko <andrii.nakryiko@gmail.com>:
>
> On Thu, Feb 27, 2020 at 6:07 PM 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 definitely
> > compared it with the size of "int" now. Therefore, errors could occur
> > when the size of the "enum" type was changed.
> >
> > Although the size of "enum" is 4-byte by default, users can change it
> > as needed (e.g., the size of the following test variable is not 4-byte
> > but 1-byte). It can be used as a struct member as below:
> >
> > enum test : char {
>
> you can't specify that in pure C, but this will work for C:
>
> struct {
> enum { X, Y, Z } __attribute__((packed)) e;
> } tmp;
>
> Please add such a selftest, as part of fixing this bug. Thanks!
>
> Otherwise logic looks good.
Thank you for kind comments!
I will add a selftest program, and submit the next version later.
Best regards,
> > X,
> > Y,
> > Z,
> > };
> >
> > struct {
> > char a;
> > enum test b;
> > char c;
> > } tmp;
> >
> > With the setup above, when I tried to load BTF, the error was given
> > as below:
> >
> > ------------------------------------------------------------------
> >
> > [58] STRUCT (anon) size=3 vlen=3
> > a type_id=55 bits_offset=0
> > b type_id=59 bits_offset=8
> > c type_id=55 bits_offset=16
> > [59] ENUM test size=1 vlen=3
> > X val=0
> > Y val=1
> > Z val=2
> >
> > [58] STRUCT (anon) size=3 vlen=3
> > b type_id=59 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 patch fixes
> > my explained issue by using the correct size of "enum" declared 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 [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-02 6:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28 2:02 [PATCH bpf] bpf: btf: Fix BTF verification of the enum size in struct/union Yoshiki Komachi
2020-02-29 0:23 ` Andrii Nakryiko
2020-03-02 6:57 ` Yoshiki Komachi
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.