Re: [PATCH v3 07/28] fsverity: always use bitmap to track verified status

From: Eric Biggers <ebiggers@kernel.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Andrey Albershteyn <aalbersh@redhat.com>,
	linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	fsverity@lists.linux.dev, djwong@kernel.org, david@fromorbit.com,
	dchinner@redhat.com, willy@infradead.org
Subject: Re: [PATCH v3 07/28] fsverity: always use bitmap to track verified status
Date: Tue, 17 Oct 2023 10:49:50 -0700	[thread overview]
Message-ID: <20231017174950.GA965@sol.localdomain> (raw)
In-Reply-To: <ZS4iWdsXQT7CaxS6@infradead.org>

On Mon, Oct 16, 2023 at 10:57:45PM -0700, Christoph Hellwig wrote:
> On Thu, Oct 12, 2023 at 12:27:46AM -0700, Eric Biggers wrote:
> > Currently there are two options: PG_checked and the separate bitmap.  I'm not
> > yet convinced that removing the support for the PG_checked method is a good
> > change.  PG_checked is a nice solution for the cases where it can be used; it
> > requires no extra memory, no locking, and has no max file size.  Also, this
> > change seems mostly orthogonal to what you're actually trying to accomplish.
> 
> Given that willy has been on a (IMHO reasonable) quest to kill off
> as many as possible page flags I'd really like to seize the opportunity
> and kill PageCheck in fsverity.  How big are the downsides of the bitmap
> vs using the page flags, and do they matter in practice?
> 

Currently PG_checked is used even with the bitmap-based approach, as a way to
ensure that hash pages that get evicted and re-instantiated from the backing
storage are re-verified.  See is_hash_block_verified() in fs/verity/verify.c.
That would need to be replaced with something else.  I'm not sure what else
would work and still be efficient.

No one has actually deployed the bitmap-based approach in production yet; it was
added in v6.3 and is only used for merkle_tree_block_size != PAGE_SIZE.  But,
the performance and memory overhead is probably not significant in practice.
I'm more worried about the file size limit of the bitmap-based approach, which
is currently ~4 TB.  *Probably* no one is using fsverity on files that large,
but introducing a limit for a case that wasn't supported before
(merkle_tree_block_size != PAGE_SIZE) isn't as bad as retroactively introducing
a limit for existing files that worked before and refusing to open them.  Huge
files would need something other than a simple bitmap, which would add
complexity and overhead.

Note that many filesystems use PG_checked for other purposes such as keeping
track of when directory blocks have been validated.  I'm not sure how feasible
it will be to free up that flag entirely.

- Eric