linux-kernel - Re: [PATCH 1/2] pipe: change pipe_write() to never add a zero-sized buffer

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-=wihBAcJLiC9dxj1M8AKHpdvrRneNk3=s-Rt-Hv5ikqo4g@mail.gmail.com>
Date: Sun, 9 Feb 2025 10:52:25 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Christian Brauner <brauner@...nel.org>, Jeff Layton <jlayton@...nel.org>, 
	David Howells <dhowells@...hat.com>, "Gautham R. Shenoy" <gautham.shenoy@....com>, 
	K Prateek Nayak <kprateek.nayak@....com>, Mateusz Guzik <mjguzik@...il.com>, 
	Neeraj Upadhyay <Neeraj.Upadhyay@....com>, Oliver Sang <oliver.sang@...el.com>, 
	Swapnil Sapkal <swapnil.sapkal@....com>, WangYuli <wangyuli@...ontech.com>, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] pipe: change pipe_write() to never add a zero-sized buffer

On Sun, 9 Feb 2025 at 10:45, Oleg Nesterov <oleg@...hat.com> wrote:
>
> Again, lets look eat_empty_buffer().
>
> The comment says "maybe it's empty" but how/why can this happen ?

WHY DO YOU CARE?

Even if it's just defensive programming, it's just plain good code,
when the rule is "the caller is waiting for data".

And if it means that you don't need to add random WARN_ON_ONCE() statements.

So here's the deal: either you

 (a) convince yourself that the length really can never be true, and
you write a comment about why that is the case.

or

 (b) you DON'T convince yourself that that is true, and you leave
eat_empty_buffer() alone.

and both of those situations are fine.

But adding a random sprinkling of WARN_ON_ONCE() statements is worse
than *either* of those two cases.

See what my argument is? Adding a WARN_ON is just making the code
worse. Don't do it. It's the worst of both worlds: it adds code to
generate, and it adds confusion to readers ("why are we warning?" or
"when can this happen").

In contrast, the "eat_empty_buffer()" case just saying "if it's an
empty buffer, it doesn't satisfy my needs, so I'll just release the
empty buffer and go on".

That's simple and straightforward. Easy to maintain, and more
efficient than your WARN_ONs.

And if you can convince yourself it's not needed, you remove it.
EXACTLY LIKE THE WARN_ON.

So there's simply never any upside to the WARN_ON.

               Linus