2020-12-28 13:47:32

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 5.4 106/453] libbpf: Fix BTF data layout checks and allow empty BTF

From: Andrii Nakryiko <[email protected]>

[ Upstream commit d8123624506cd62730c9cd9c7672c698e462703d ]

Make data section layout checks stricter, disallowing overlap of types and
strings data.

Additionally, allow BTFs with no type data. There is nothing inherently wrong
with having BTF with no types (put potentially with some strings). This could
be a situation with kernel module BTFs, if module doesn't introduce any new
type information.

Also fix invalid offset alignment check for btf->hdr->type_off.

Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
Signed-off-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
Signed-off-by: Sasha Levin <[email protected]>
---
tools/lib/bpf/btf.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index d606a358480da..3380aadb74655 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -100,22 +100,18 @@ static int btf_parse_hdr(struct btf *btf)
return -EINVAL;
}

- if (meta_left < hdr->type_off) {
- pr_debug("Invalid BTF type section offset:%u\n", hdr->type_off);
+ if (meta_left < hdr->str_off + hdr->str_len) {
+ pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
return -EINVAL;
}

- if (meta_left < hdr->str_off) {
- pr_debug("Invalid BTF string section offset:%u\n", hdr->str_off);
+ if (hdr->type_off + hdr->type_len > hdr->str_off) {
+ pr_debug("Invalid BTF data sections layout: type data at %u + %u, strings data at %u + %u\n",
+ hdr->type_off, hdr->type_len, hdr->str_off, hdr->str_len);
return -EINVAL;
}

- if (hdr->type_off >= hdr->str_off) {
- pr_debug("BTF type section offset >= string section offset. No type?\n");
- return -EINVAL;
- }
-
- if (hdr->type_off & 0x02) {
+ if (hdr->type_off % 4) {
pr_debug("BTF type section is not aligned to 4 bytes\n");
return -EINVAL;
}
--
2.27.0




2020-12-28 15:54:14

by Naresh Kamboju

[permalink] [raw]
Subject: Re: [PATCH 5.4 106/453] libbpf: Fix BTF data layout checks and allow empty BTF

Perf build failed on stable-rc 5.4 branch due to this patch.

On Mon, 28 Dec 2020 at 19:15, Greg Kroah-Hartman
<[email protected]> wrote:
>
> From: Andrii Nakryiko <[email protected]>
>
> [ Upstream commit d8123624506cd62730c9cd9c7672c698e462703d ]
>
> Make data section layout checks stricter, disallowing overlap of types and
> strings data.
>
> Additionally, allow BTFs with no type data. There is nothing inherently wrong
> with having BTF with no types (put potentially with some strings). This could
> be a situation with kernel module BTFs, if module doesn't introduce any new
> type information.
>
> Also fix invalid offset alignment check for btf->hdr->type_off.
>
> Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
> Signed-off-by: Andrii Nakryiko <[email protected]>
> Signed-off-by: Alexei Starovoitov <[email protected]>
> Link: https://lore.kernel.org/bpf/[email protected]
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> tools/lib/bpf/btf.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index d606a358480da..3380aadb74655 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -100,22 +100,18 @@ static int btf_parse_hdr(struct btf *btf)
> return -EINVAL;
> }
>
> - if (meta_left < hdr->type_off) {
> - pr_debug("Invalid BTF type section offset:%u\n", hdr->type_off);
> + if (meta_left < hdr->str_off + hdr->str_len) {
> + pr_debug("Invalid BTF total size:%u\n", btf->raw_size);

In file included from btf.c:17:0:
btf.c: In function 'btf_parse_hdr':
btf.c:104:48: error: 'struct btf' has no member named 'raw_size'; did
you mean 'data_size'?
pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
^
libbpf_internal.h:59:40: note: in definition of macro '__pr'
libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
btf.c:104:3: note: in expansion of macro 'pr_debug'
pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
^~~~~~~~

full build log link,
https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.4/DISTRO=lkft,MACHINE=intel-corei7-64,label=docker-buster-lkft/346/consoleText

--
Linaro LKFT
https://lkft.linaro.org

2020-12-29 01:39:25

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 5.4 106/453] libbpf: Fix BTF data layout checks and allow empty BTF

On Mon, Dec 28, 2020 at 11:47:44AM -0800, Andrii Nakryiko wrote:
>On Mon, Dec 28, 2020 at 7:49 AM Naresh Kamboju
><[email protected]> wrote:
>>
>> Perf build failed on stable-rc 5.4 branch due to this patch.
>>
>> On Mon, 28 Dec 2020 at 19:15, Greg Kroah-Hartman
>> <[email protected]> wrote:
>> >
>> > From: Andrii Nakryiko <[email protected]>
>> >
>> > [ Upstream commit d8123624506cd62730c9cd9c7672c698e462703d ]
>> >
>> > Make data section layout checks stricter, disallowing overlap of types and
>> > strings data.
>> >
>> > Additionally, allow BTFs with no type data. There is nothing inherently wrong
>> > with having BTF with no types (put potentially with some strings). This could
>> > be a situation with kernel module BTFs, if module doesn't introduce any new
>> > type information.
>> >
>> > Also fix invalid offset alignment check for btf->hdr->type_off.
>> >
>> > Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
>> > Signed-off-by: Andrii Nakryiko <[email protected]>
>> > Signed-off-by: Alexei Starovoitov <[email protected]>
>> > Link: https://lore.kernel.org/bpf/[email protected]
>> > Signed-off-by: Sasha Levin <[email protected]>
>> > ---
>> > tools/lib/bpf/btf.c | 16 ++++++----------
>> > 1 file changed, 6 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>> > index d606a358480da..3380aadb74655 100644
>> > --- a/tools/lib/bpf/btf.c
>> > +++ b/tools/lib/bpf/btf.c
>> > @@ -100,22 +100,18 @@ static int btf_parse_hdr(struct btf *btf)
>> > return -EINVAL;
>> > }
>> >
>> > - if (meta_left < hdr->type_off) {
>> > - pr_debug("Invalid BTF type section offset:%u\n", hdr->type_off);
>> > + if (meta_left < hdr->str_off + hdr->str_len) {
>> > + pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
>>
>> In file included from btf.c:17:0:
>> btf.c: In function 'btf_parse_hdr':
>> btf.c:104:48: error: 'struct btf' has no member named 'raw_size'; did
>> you mean 'data_size'?
>> pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
>> ^
>> libbpf_internal.h:59:40: note: in definition of macro '__pr'
>> libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
>> ^~~~~~~~~~~
>> btf.c:104:3: note: in expansion of macro 'pr_debug'
>> pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
>> ^~~~~~~~
>>
>
>This patch fixes the bug introduced back in 8a138aed4a80 ("bpf: btf:
>Add BTF support to libbpf"), but a bunch of other refactorings
>happened since then, adding/renaming struct btf internals. The fix
>here is not that critical, so it's probably best to just drop this
>patch from the stable, if possible. Would it be ok, Greg?

I'll drop it, thanks.

--
Thanks,
Sasha

2020-12-29 01:45:15

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 5.4 106/453] libbpf: Fix BTF data layout checks and allow empty BTF

On Mon, Dec 28, 2020 at 7:49 AM Naresh Kamboju
<[email protected]> wrote:
>
> Perf build failed on stable-rc 5.4 branch due to this patch.
>
> On Mon, 28 Dec 2020 at 19:15, Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > From: Andrii Nakryiko <[email protected]>
> >
> > [ Upstream commit d8123624506cd62730c9cd9c7672c698e462703d ]
> >
> > Make data section layout checks stricter, disallowing overlap of types and
> > strings data.
> >
> > Additionally, allow BTFs with no type data. There is nothing inherently wrong
> > with having BTF with no types (put potentially with some strings). This could
> > be a situation with kernel module BTFs, if module doesn't introduce any new
> > type information.
> >
> > Also fix invalid offset alignment check for btf->hdr->type_off.
> >
> > Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
> > Signed-off-by: Andrii Nakryiko <[email protected]>
> > Signed-off-by: Alexei Starovoitov <[email protected]>
> > Link: https://lore.kernel.org/bpf/[email protected]
> > Signed-off-by: Sasha Levin <[email protected]>
> > ---
> > tools/lib/bpf/btf.c | 16 ++++++----------
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index d606a358480da..3380aadb74655 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -100,22 +100,18 @@ static int btf_parse_hdr(struct btf *btf)
> > return -EINVAL;
> > }
> >
> > - if (meta_left < hdr->type_off) {
> > - pr_debug("Invalid BTF type section offset:%u\n", hdr->type_off);
> > + if (meta_left < hdr->str_off + hdr->str_len) {
> > + pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
>
> In file included from btf.c:17:0:
> btf.c: In function 'btf_parse_hdr':
> btf.c:104:48: error: 'struct btf' has no member named 'raw_size'; did
> you mean 'data_size'?
> pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
> ^
> libbpf_internal.h:59:40: note: in definition of macro '__pr'
> libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
> ^~~~~~~~~~~
> btf.c:104:3: note: in expansion of macro 'pr_debug'
> pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
> ^~~~~~~~
>

This patch fixes the bug introduced back in 8a138aed4a80 ("bpf: btf:
Add BTF support to libbpf"), but a bunch of other refactorings
happened since then, adding/renaming struct btf internals. The fix
here is not that critical, so it's probably best to just drop this
patch from the stable, if possible. Would it be ok, Greg?

> full build log link,
> https://ci.linaro.org/view/lkft/job/openembedded-lkft-linux-stable-rc-5.4/DISTRO=lkft,MACHINE=intel-corei7-64,label=docker-buster-lkft/346/consoleText
>
> --
> Linaro LKFT
> https://lkft.linaro.org

2020-12-29 09:46:02

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 5.4 106/453] libbpf: Fix BTF data layout checks and allow empty BTF

On Mon, Dec 28, 2020 at 06:09:25PM -0500, Sasha Levin wrote:
> On Mon, Dec 28, 2020 at 11:47:44AM -0800, Andrii Nakryiko wrote:
> > On Mon, Dec 28, 2020 at 7:49 AM Naresh Kamboju
> > <[email protected]> wrote:
> > >
> > > Perf build failed on stable-rc 5.4 branch due to this patch.
> > >
> > > On Mon, 28 Dec 2020 at 19:15, Greg Kroah-Hartman
> > > <[email protected]> wrote:
> > > >
> > > > From: Andrii Nakryiko <[email protected]>
> > > >
> > > > [ Upstream commit d8123624506cd62730c9cd9c7672c698e462703d ]
> > > >
> > > > Make data section layout checks stricter, disallowing overlap of types and
> > > > strings data.
> > > >
> > > > Additionally, allow BTFs with no type data. There is nothing inherently wrong
> > > > with having BTF with no types (put potentially with some strings). This could
> > > > be a situation with kernel module BTFs, if module doesn't introduce any new
> > > > type information.
> > > >
> > > > Also fix invalid offset alignment check for btf->hdr->type_off.
> > > >
> > > > Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
> > > > Signed-off-by: Andrii Nakryiko <[email protected]>
> > > > Signed-off-by: Alexei Starovoitov <[email protected]>
> > > > Link: https://lore.kernel.org/bpf/[email protected]
> > > > Signed-off-by: Sasha Levin <[email protected]>
> > > > ---
> > > > tools/lib/bpf/btf.c | 16 ++++++----------
> > > > 1 file changed, 6 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > > index d606a358480da..3380aadb74655 100644
> > > > --- a/tools/lib/bpf/btf.c
> > > > +++ b/tools/lib/bpf/btf.c
> > > > @@ -100,22 +100,18 @@ static int btf_parse_hdr(struct btf *btf)
> > > > return -EINVAL;
> > > > }
> > > >
> > > > - if (meta_left < hdr->type_off) {
> > > > - pr_debug("Invalid BTF type section offset:%u\n", hdr->type_off);
> > > > + if (meta_left < hdr->str_off + hdr->str_len) {
> > > > + pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
> > >
> > > In file included from btf.c:17:0:
> > > btf.c: In function 'btf_parse_hdr':
> > > btf.c:104:48: error: 'struct btf' has no member named 'raw_size'; did
> > > you mean 'data_size'?
> > > pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
> > > ^
> > > libbpf_internal.h:59:40: note: in definition of macro '__pr'
> > > libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
> > > ^~~~~~~~~~~
> > > btf.c:104:3: note: in expansion of macro 'pr_debug'
> > > pr_debug("Invalid BTF total size:%u\n", btf->raw_size);
> > > ^~~~~~~~
> > >
> >
> > This patch fixes the bug introduced back in 8a138aed4a80 ("bpf: btf:
> > Add BTF support to libbpf"), but a bunch of other refactorings
> > happened since then, adding/renaming struct btf internals. The fix
> > here is not that critical, so it's probably best to just drop this
> > patch from the stable, if possible. Would it be ok, Greg?
>
> I'll drop it, thanks.

Thanks for dropping this and doing the other fixups. I'll go push out
some -rc2 releases soon...

greg k-h