Re: [PATCH bpf] bpf: add check for invalid name in btf_name_valid_section()

[Eduard Zingerman]

On Thu, 2024-08-29 at 12:45 +0900, Jeongjun Park wrote:
> Alexei Starovoitov wrote:
> > 
> > On Fri, Aug 23, 2024 at 3:43 AM Jeongjun Park <aha310510@xxxxxxxxx> wrote:
> > > 
> > > If the length of the name string is 1 and the value of name[0] is NULL
> > > byte, an OOB vulnerability occurs in btf_name_valid_section() and the
> > > return value is true, so the invalid name passes the check.
> > > 
> > > To solve this, you need to check if the first position is NULL byte.
> > > 
> > > Fixes: bd70a8fb7ca4 ("bpf: Allow all printable characters in BTF DATASEC names")
> > > Signed-off-by: Jeongjun Park <aha310510@xxxxxxxxx>
> > > ---
> > >  kernel/bpf/btf.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > > index 520f49f422fe..5c24ea1a65a4 100644
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -823,6 +823,9 @@ static bool btf_name_valid_section(const struct btf *btf, u32 offset)
> > >         const char *src = btf_str_by_offset(btf, offset);
> > >         const char *src_limit;
> > > 
> > > +       if (!*src)
> > > +               return false;
> > > +
> > 
> > We've talked about it. Quote:
> > "Pls add a selftest that demonstrates the issue
> > and produce a patch to fix just that."
> > 
> > length == 1 and name[0] = 0 is a hypothesis.
> > Demonstrate that such a scenario is possible then this patch will be
> > worth applying.
> > 
> > pw-bot: cr
> 
> Sorry for the omission, I still don't know how to write selftest.
> 
> But I can give you the C repro and KASAN log that trigger this vulnerability. 
> I would appreciate it if you could look at it and make a judgment.

I will prepare a test case.
Probably tomorrow.