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@xxxxxxxxxx Will
