On Wed, Nov 3, 2021 at 9:36 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Tue, Nov 2, 2021 at 11:06 PM Yonghong Song <yhs@xxxxxx> wrote:
> >
> >
> >
> > On 11/2/21 5:10 PM, Andrii Nakryiko wrote:
> > > If BTF is corrupted DATASEC's variable type ID might be incorrect.
> > > Prevent this easy to detect situation with extra NULL check.
> > > Reported by oss-fuzz project.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> >
> > Ack with a nit below.
> > Acked-by: Yonghong Song <yhs@xxxxxx>
> >
> > > ---
> > > tools/lib/bpf/libbpf.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 71f5a009010a..4537ce6d54ce 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -2754,7 +2754,7 @@ static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
> > > t_var = btf__type_by_id(btf, vsi->type);
> > > var = btf_var(t_var);
> >
> > Can we move the above 'var = ...' assignment after below if statement?
>
> it's safe as is because btf_var() is equivalent to pointer casting. I
> considered doing a check before btf_var() cast, but that would require
> a separate if and pr_debug statements which felt like an overkill.
Oh, never mind, we don't validate var itself, so no need for extra if.
I'll post a v2 with this change.
>
> >
> > >
> > > - if (!btf_is_var(t_var)) {
> > > + if (!t_var || !btf_is_var(t_var)) {
> > > pr_debug("Non-VAR type seen in section %s\n", name);
> > > return -EINVAL;
> > > }
> > >
