Re: [PATCH v1] mm/gup: fix FOLL_FORCE COW security issue and remove FOLL_COW

From: Linus Torvalds <torvalds@linux-foundation.org>
To: David Hildenbrand <david@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	stable@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Peter Xu <peterx@redhat.com>, Hugh Dickins <hughd@google.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	John Hubbard <jhubbard@nvidia.com>,
	Jason Gunthorpe <jgg@nvidia.com>
Subject: Re: [PATCH v1] mm/gup: fix FOLL_FORCE COW security issue and remove FOLL_COW
Date: Tue, 9 Aug 2022 11:59:45 -0700	[thread overview]
Message-ID: <CAHk-=wg40EAZofO16Eviaj7mfqDhZ2gVEbvfsMf6gYzspRjYvw@mail.gmail.com> (raw)
In-Reply-To: <1a48d71d-41ee-bf39-80d2-0102f4fe9ccb@redhat.com>

On Tue, Aug 9, 2022 at 11:45 AM David Hildenbrand <david@redhat.com> wrote:
>
> I totally agree with BUG_ON ... but if I get talked to in all-caps on a
> Thursday evening and feel like I just touched the forbidden fruit, I
> have to ask for details about VM_BUG_ON nowadays.
>
> VM_BUG_ON is only active with CONFIG_DEBUG_VM. ... which indicated some
> kind of debugging at least to me. I *know* that Fedora enables it and I
> *know* that this will make Fedora crash.

No.

VM_BUG_ON() has the exact same semantics as BUG_ON. It is literally no
different, the only difference is "we can make the code smaller
because these are less important".

The only possible case where BUG_ON can validly be used is "I have
some fundamental data corruption and cannot possibly return an error".

This kind of "I don't think this can happen" is _never_ an excuse for it.

Honestly, 99% of our existing BUG_ON() ones are completely bogus, and
left-over debug code that wasn't removed because they never triggered.
I've several times considered just using a coccinelle script to remove
every single BUG_ON() (and VM_BUG_ON()) as simply bogus. Because they
are pure noise.

I just tried to find a valid BUG_ON() that would make me go "yeah,
that's actually worth it", and couldn't really find one. Yeah, there
are several ones in the scheduler that make me go "ok, if that
triggers, the machine is dead anyway", so in that sense there are
certainly BUG_ON()s that don't _hurt_.

But as a very good approximation, the rule is "absolutely no new
BUG_ON() calls _ever_". Because I really cannot see a single case
where "proper error handling and WARN_ON_ONCE()" isn't the right
thing.

Now, that said, there is one very valid sub-form of BUG_ON():
BUILD_BUG_ON() is absolutely 100% fine.

                Linus