linux-kernel - Re: [PATCH] lib/string_helpers: rework overflow-dependent code

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: <CAHp75Vd3wKyq2XE2UPoW_q3KjmncSeaEebL4ff5Gpx8Lz+dB9A@mail.gmail.com>
Date: Fri, 9 Aug 2024 14:07:57 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Kees Cook <kees@...nel.org>
Cc: Justin Stitt <justinstitt@...gle.com>, Andy Shevchenko <andy@...nel.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org, 
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH] lib/string_helpers: rework overflow-dependent code

On Fri, Aug 9, 2024 at 2:11 AM Kees Cook <kees@...nel.org> wrote:
>
> On Fri, Aug 09, 2024 at 01:07:21AM +0300, Andy Shevchenko wrote:
> > On Fri, Aug 9, 2024 at 12:44 AM Justin Stitt <justinstitt@...gle.com> wrote:
> > >
> > > When @size is 0, the desired behavior is to allow unlimited bytes to be
> > > parsed. Currently, this relies on some intentional arithmetic overflow
> > > where --size gives us SIZE_MAX when size is 0.
> > >
> > > Explicitly spell out the desired behavior without relying on intentional
> > > overflow/underflow.
> >
> > Hmm... but why? Overflow for the _unsigned_ types is okay. No?
>
> Yes, it's well defined, but in trying to find a place to start making a
> meaningful impact on unexpected wrap-around, after discussions with
> Linus and Peter Zijlstra, we're going taking a stab at defining size_t
> as not expecting to wrap. Justin has been collecting false positive
> fixes while working on the compiler side of this, and I had asked him to
> send this one now since I think it additionally helps with readability.

Okay, but the patch has an off-by-one error (which has no impact on
the behavior as it's close to unrealistic to have the SIZE_MAX array).
I prefer that patch can be reconsidered to keep original behaviour,
otherwise it might be not so clear why 0 is SIZE_MAX - 1 in _this_
case.

-- 
With Best Regards,
Andy Shevchenko