linux-hardening - Re: [PATCH 34/82] ipc: Refactor intentional wrap-around calculation

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: <CAHk-=wgMPVv-mDxA2qcywpLCRLojtaKmP13h7bVo4m=XN202xA@mail.gmail.com>
Date: Tue, 23 Jan 2024 10:06:12 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Kees Cook <keescook@...omium.org>
Cc: linux-hardening@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, Mark Brown <broonie@...nel.org>, 
	Mike Kravetz <mike.kravetz@...cle.com>, Vasily Averin <vasily.averin@...ux.dev>, 
	Alexander Mikhalitsyn <alexander@...alicyn.com>, "Gustavo A. R. Silva" <gustavoars@...nel.org>, 
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 34/82] ipc: Refactor intentional wrap-around calculation

On Mon, 22 Jan 2024 at 17:38, Kees Cook <keescook@...omium.org> wrote:
>
> I've tried to find the right balance between not enough details and too
> much. I guess I got it wrong.

My complaint isn't about the level of detail.

My complaint is about how the commit log IS ACTIVELY MISLEADING
GARBAGE and does not match the actual patch in any way, shape, or
form.

It talks about completely irrelevant issues that simply have nothing
to do with it.

It talks about undefined behavior and about a "unsigned wrap-around
sanitizer[2]", which is nonsensical, since there is no undefined
behavior to sanitize. It literally gives a link to a github "issue"
for that claim, but when you follow the link, it's actually about
*signed* overflow, which is something entirely different.

And honestly, the patch itself is garbage. The code is fine. Any
"sanitizer" that complains about that code is pure and utter shite.

Really.

If you actually have some real "detect unsigned wraparound" tool
(NOTE: that is *NOT* undefined behavior, and that is *NOT* a
"sanitizer", it's at most some helpful checker), then such a tool had
better recognize the perfectly fine traditional idiom for this, which
is to do the addition and check that the result is smaller. Like the
code does.

See what I'm saying? The patch is garbage. Any sanitizer that would
complain about the old code is garbage. And the commit message is
worse than garbage, it is actively misleading to the point that I'd
call it lying, trying to confuse the issues by bringing up things that
are utterly and entirely irrelevant to the patch.

So:

 - get rid of that commit message that is lying garbage

 - fix the so-called "sanitizer".

 - stop calling the unsigned wrap-around a "sanitizer" and talking
about "undefined behavior" in the same sentence, since it's neither.

Do you really not see why I think that thing is actively *WRONG*?

           Linus