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: <ZvmxntpAHrAwcKaH@archlinux>
Date: Sun, 29 Sep 2024 21:59:26 +0200
From: Jan Hendrik Farr <kernel@...rr.cc>
To: Kees Cook <kees@...nel.org>
Cc: Bill Wendling <morbo@...gle.com>, Ard Biesheuvel <ardb@...nel.org>,
	Thorsten Blum <thorsten.blum@...lux.com>, kent.overstreet@...ux.dev,
	regressions@...ts.linux.dev, linux-bcachefs@...r.kernel.org,
	linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [REGRESSION][BISECTED] erroneous buffer overflow detected in
 bch2_xattr_validate

On 29 01:33:40, Jan Hendrik Farr wrote:
> On 28 13:50:12, Kees Cook wrote:
> > On Thu, Sep 26, 2024 at 06:30:15PM -0700, Bill Wendling wrote:
> > > On Thu, Sep 26, 2024 at 3:18 PM Bill Wendling <morbo@...gle.com> wrote:
> > > >
> > > > On Thu, Sep 26, 2024 at 12:58 PM Ard Biesheuvel <ardb@...nel.org> wrote:
> > > > >
> > > > > (cc Kees and Bill)
> > > > >
> > > > > On Thu, 26 Sept 2024 at 19:46, Jan Hendrik Farr <kernel@...rr.cc> wrote:
> > > > > >
> > > > > > On 26 19:01:20, Jan Hendrik Farr wrote:
> > > > > > > On 26 18:09:57, Thorsten Blum wrote:
> > > > > > > > On 26. Sep 2024, at 17:28, Thorsten Blum <thorsten.blum@...lux.com> wrote:
> > > > > > > > > On 26. Sep 2024, at 17:14, Jan Hendrik Farr <kernel@...rr.cc> wrote:
> > > [...]
> > > > > > > > >> [    6.252375] memchr: detected buffer overflow: 12 byte read of buffer size 0
> > > [...]
> > > > > > >       bkey_fsck_err_on(memchr(xattr.v->x_name, '\0', xattr.v->x_name_len),
> > > > > > >                        c, xattr_name_invalid_chars,
> > > > > > >                        "xattr name has invalid characters");
> > > [...]
> > 
> > The thing going wrong is that __bdos(xattr.v->x_name, 0) is returning 0.
> > This looks exactly like the bug I minimized here:
> > https://lore.kernel.org/all/202409170436.C3C6E7F7A@keescook/
> > 
> > Since there wasn't an LLVM open bug yet, I've created:
> > https://github.com/llvm/llvm-project/issues/110385
> > 
> > -Kees
> > 
> I found a fix for the issue. Fixes both the issue in this thread as well
> as your reproducer. First thought they might not actually be the same
> issue, but they indeed are. Haven't tested against the issue Thorsten
> linked.
> 
> Havn't run the clang tests on it yet, but it does successfully compile
> my kernel and fix the issue.
> 
> I'll open a PR and give more explanation tomorrow, it's getting
> pretty late over here in CEST.
> 
> 
> Here's the patch to be applied on top of
> https://github.com/llvm/llvm-project
> 
> --
> diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
> index 9166db4c7412..143dd3fcfcf8 100644
> --- a/clang/lib/CodeGen/CGExpr.cpp
> +++ b/clang/lib/CodeGen/CGExpr.cpp
> @@ -1164,15 +1164,15 @@ llvm::Value *CodeGenFunction::EmitLoadOfCountedByField(
>      Res = EmitDeclRefLValue(DRE).getPointer(*this);
>      Res = Builder.CreateAlignedLoad(ConvertType(DRE->getType()), Res,
>                                      getPointerAlign(), "dre.load");
> -  } else if (const MemberExpr *ME = dyn_cast<MemberExpr>(StructBase)) {
> -    LValue LV = EmitMemberExpr(ME);
> -    Address Addr = LV.getAddress();
> -    Res = Addr.emitRawPointer(*this);
>    } else if (StructBase->getType()->isPointerType()) {
>      LValueBaseInfo BaseInfo;
>      TBAAAccessInfo TBAAInfo;
>      Address Addr = EmitPointerWithAlignment(StructBase, &BaseInfo, &TBAAInfo);
>      Res = Addr.emitRawPointer(*this);
> +  } else if (const MemberExpr *ME = dyn_cast<MemberExpr>(StructBase)) {
> +    LValue LV = EmitMemberExpr(ME);
> +    Address Addr = LV.getAddress();
> +    Res = Addr.emitRawPointer(*this);
>    } else {
>      return nullptr;
>    }


Here's the PR: https://github.com/llvm/llvm-project/pull/110437

I hope the way I added the CHECK tags in the test is good and that they if
they don't need manual cleanup, not familiar with the llvm test system.


Best Regards
Jan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ