linux-kernel - Re: [BUG kernel-5.15] aarch64: __pi

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] [day] [month] [year] [list]

Message-ID: <20230906180713.GA5021@willie-the-truck>
Date:   Wed, 6 Sep 2023 19:07:13 +0100
From:   Will Deacon <will@...nel.org>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Mark Rutland <mark.rutland@....com>,
        John Hsu (許永翰) <John.Hsu@...iatek.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Xiaobing Shi (史小兵) 
        <Xiaobing.Shi@...iatek.com>,
        Chunhui Li (李春辉) 
        <chunhui.li@...iatek.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Kuan-Ying Lee (李冠穎) 
        <Kuan-Ying.Lee@...iatek.com>,
        Casper Li (李中榮) <casper.li@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [BUG kernel-5.15] aarch64: __pi_strncmp() out-of-bound error

On Thu, Aug 10, 2023 at 05:09:07PM +0100, Will Deacon wrote:
> On Thu, Aug 10, 2023 at 04:00:00PM +0100, Robin Murphy wrote:
> > On 10/08/2023 3:31 pm, Will Deacon wrote:
> > > On Thu, Aug 10, 2023 at 01:23:28PM +0100, Robin Murphy wrote:
> > > > I'm not sure there's strictly a bug here. The C standard says:
> > > > 
> > > > "The strncmp function compares not more than n characters (characters that
> > > > follow a null character are not compared) ..."
> > > > 
> > > > so although any characters between the first NULL and n must not be
> > > > considered for the result of the comparison, there doesn't seem to be any
> > > > explicit promise anywhere that they can't be *accessed*. AFAICT what happens
> > > > here is in the request to compare at most 23 characters, it ends up in the
> > > > do_misaligned case, loop_misaligned runs twice and finds no differences or
> > > > NULLs in characters 0-7 and 8-15, so then done_loop loads characters 15-23
> > > > to compare the last 7, and is tripped up by 22-23 not actually existing in
> > > > src2. Possibly the original intent was that this case should have ended up
> > > > in page_end_loop, and the condition for that was slightly off, but I'm not
> > > > sure, and this code is obsolete now anyway.
> > > 
> > > The long backtrace above worries me, as it suggests that you can trigger
> > > this from userspace. In that case I think it's a bug regardless of what
> > > the C standard says.
> > 
> > Bleh, poor choice of words... obviously there is a bug overall, it just
> > might arguably be in the caller's expectations rather than the strncmp()
> > implementation itself. However I would concur that there's no way we're
> > going over all ~3000 strncmp() callsites with the "well, actually" comb just
> > for this. It was more to say I don't think it's worth digging much deeper
> > into exactly why, and I agree the pragmatic thing to do is either rip it out
> > or backport the newer MTE-safe implementation which should be more robust.
> 
> Heh, then we agree. I was worried you'd gone mad :)

In the end I cherry-picked the newer implementation rather than fall back to
the generic implementation:

https://lore.kernel.org/r/20230906180336.4973-1-will@kernel.org

Will