Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 4 Jan 2023 12:08:09 +0530
From:   Ojaswin Mujoo <ojaswin@linux.ibm.com>
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net,
        linux-btrfs@vger.kernel.org, linux-xfs@vger.kernel.org,
        Andrey Albershteyn <aalbersh@redhat.com>
Subject: Re: [PATCH v2 00/11] fsverity: support for non-4K pages
Message-ID: <Y7UeuYVkyy2/fWF1@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com>
References: <20221223203638.41293-1-ebiggers@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221223203638.41293-1-ebiggers@kernel.org>
MIME-Version: 1.0
Precedence: bulk

Hi Eric,

I have roughly gone through the series and run the (patched) xfstests on
this patchset on a powerpc machine with 64k pagesize and 64k,4k and 1k
merkle tree size on EXT4 and everything seems to work correctly. 

Just for records, test generic/692 takes a lot of time to complete with
64k merkel tree size due to the calculations assuming it to be 4k,
however I was able to manually test that particular scenario. (I'll try
to send a patch to fix the fstest later).

Anyways, feel free to add:

Tested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Since I was not very familiar with the fsverty codebase, I'll try to
take some more time to review the code and get back with any
comments/RVBs.

Regards,
ojaswin

On Fri, Dec 23, 2022 at 12:36:27PM -0800, Eric Biggers wrote:
> [This patchset applies to mainline + some fsverity cleanups I sent out
>  recently.  You can get everything from tag "fsverity-non4k-v2" of
>  https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git ]
> 
> Currently, filesystems (ext4, f2fs, and btrfs) only support fsverity
> when the Merkle tree block size, filesystem block size, and page size
> are all the same.  In practice that means 4K, since increasing the page
> size, e.g. to 16K, forces the Merkle tree block size and filesystem
> block size to be increased accordingly.  That can be impractical; for
> one, users want the same file signatures to work on all systems.
> 
> Therefore, this patchset reduces the coupling between these sizes.
> 
> First, patches 1-4 are cleanups.
> 
> Second, patches 5-9 allow the Merkle tree block size to be less than the
> page size or filesystem block size, provided that it's not larger than
> either one.  This involves, among other things, changing the way that
> fs/verity/verify.c tracks which hash blocks have been verified.
> 
> Finally, patches 10-11 make ext4 support fsverity when the filesystem
> block size is less than the page size.  Note, f2fs doesn't need similar
> changes because f2fs always assumes that the filesystem block size and
> page size are the same anyway.  I haven't looked into btrfs yet.
> 
> I've tested this patchset using the "verity" group of tests in xfstests
> with the following xfstests patchset applied:
> "[PATCH v2 00/10] xfstests: update verity tests for non-4K block and page size"
> (https://lore.kernel.org/fstests/20221223010554.281679-1-ebiggers@kernel.org/T/#u)
> 
> Note: on the thread "[RFC PATCH 00/11] fs-verity support for XFS"
> (https://lore.kernel.org/linux-xfs/20221213172935.680971-1-aalbersh@redhat.com/T/#u)
> there have been many requests for other things to support, including:
> 
>   * folios in the pagecache
>   * alternative Merkle tree caching methods
>   * direct I/O
>   * merkle_tree_block_size > page_size
>   * extremely large files, using a reclaimable bitmap
> 
> We shouldn't try to boil the ocean, though, so to keep the scope of this
> patchset manageable I haven't changed it significantly from v1.  This
> patchset does bring us closer to many of the above, just not all the way
> there.  I'd like to follow up this patchset with a change to support
> folios, which should be straightforward.  Next, we can do a change to
> generalize the Merkle tree interface to allow XFS to use an alternative
> caching method, as that sounds like the highest priority item for XFS.
> 
> Anyway, the changelog is:
> 
> Changed in v2:
>    - Rebased onto the recent fsverity cleanups.
>    - Split some parts of the big "support verification" patch into
>      separate patches.
>    - Passed the data_pos to verify_data_block() instead of computing it
>      using page->index, to make it ready for folio and DIO support.
>    - Eliminated some unnecessary arithmetic in verify_data_block().
>    - Changed the log_* fields in merkle_tree_params to u8.
>    - Restored PageLocked and !PageUptodate checks for pagecache pages.
>    - Eliminated the change to fsverity_hash_buffer().
>    - Other small cleanups
> 
> Eric Biggers (11):
>   fsverity: use unsigned long for level_start
>   fsverity: simplify Merkle tree readahead size calculation
>   fsverity: store log2(digest_size) precomputed
>   fsverity: use EFBIG for file too large to enable verity
>   fsverity: replace fsverity_hash_page() with fsverity_hash_block()
>   fsverity: support verification with tree block size < PAGE_SIZE
>   fsverity: support enabling with tree block size < PAGE_SIZE
>   ext4: simplify ext4_readpage_limit()
>   f2fs: simplify f2fs_readpage_limit()
>   fs/buffer.c: support fsverity in block_read_full_folio()
>   ext4: allow verity with fs block size < PAGE_SIZE
> 
>  Documentation/filesystems/fsverity.rst |  76 +++---
>  fs/buffer.c                            |  67 ++++-
>  fs/ext4/readpage.c                     |   3 +-
>  fs/ext4/super.c                        |   5 -
>  fs/f2fs/data.c                         |   3 +-
>  fs/verity/enable.c                     | 260 ++++++++++----------
>  fs/verity/fsverity_private.h           |  20 +-
>  fs/verity/hash_algs.c                  |  24 +-
>  fs/verity/open.c                       |  98 ++++++--
>  fs/verity/verify.c                     | 325 +++++++++++++++++--------
>  include/linux/fsverity.h               |  14 +-
>  11 files changed, 565 insertions(+), 330 deletions(-)
> 
> -- 
> 2.39.0
>