linux-kernel - Re: [PATCH bpf] bpf: add check for invalid name in btf_name_valid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d1ca563d8f2f5b63e7b0ec8b91c57914c32f1679.camel@gmail.com>
Date: Wed, 28 Aug 2024 22:45:18 -0700
From: Eduard Zingerman <eddyz87@...il.com>
To: Jeongjun Park <aha310510@...il.com>, alexei.starovoitov@...il.com
Cc: andrii@...nel.org, ast@...nel.org, bpf@...r.kernel.org,
 daniel@...earbox.net,  haoluo@...gle.com, john.fastabend@...il.com,
 jolsa@...nel.org, kpsingh@...nel.org,  linux-kernel@...r.kernel.org,
 martin.lau@...ux.dev, sdf@...ichev.me,  song@...nel.org,
 yonghong.song@...ux.dev
Subject: Re: [PATCH bpf] bpf: add check for invalid name in
 btf_name_valid_section()

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@...il.com> 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@...il.com>
> > > ---
> > >  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.