| Message ID | 20221223203638.41293-1-ebiggers@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | fsverity: support for non-4K pages | expand |
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 >
On Wed, Jan 04, 2023 at 12:08:09PM +0530, Ojaswin Mujoo wrote: > 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 Thanks Ojaswin! That's a good point about generic/692. The right fix for it is to make it use $FSV_BLOCK_SIZE instead of 4K in its calculations. I suppose you saw that issue by running the test on ext4 with fs_block_size == page_size == 64K, causing xfstests to use merkle_tree_block_size == 64K by default. Thanks for doing that; that's something I haven't been able to test yet. My focus has been on merkle_tree_block_size < page_size. merkle_tree_block_size > 4K should just work, though, assuming merkle_tree_block_size <= min(fs_block_size, page_size). (Or merkle_tree_block_size == fs_block_size == page_size before this patch series.) - Eric
On Tue, Jan 03, 2023 at 11:25:18PM -0800, Eric Biggers wrote: > Hi Eric, > Thanks Ojaswin! That's a good point about generic/692. The right fix for it is > to make it use $FSV_BLOCK_SIZE instead of 4K in its calculations. Yes, that should fix the issue, I'll try to send in a patch for this when I find some time. > > I suppose you saw that issue by running the test on ext4 with fs_block_size == > page_size == 64K, causing xfstests to use merkle_tree_block_size == 64K by > default. Thanks for doing that; that's something I haven't been able to test > yet. My focus has been on merkle_tree_block_size < page_size. Correct, I was testing "everything = 64k" scenario when I noticed the slowdown. > merkle_tree_block_size > 4K should just work, though, assuming > merkle_tree_block_size <= min(fs_block_size, page_size). (Or > merkle_tree_block_size == fs_block_size == page_size before this patch series.) Yes true, I still tested them just in case :) 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 ] I've applied this patchset for 6.3, but I'd still greatly appreciate reviews and acks, especially on the last 4 patches, which touch files outside fs/verity/. (I applied it to https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git/log/?h=fsverity for now, but there might be a new git repo soon, as is being discussed elsewhere.) - Eric
On Mon, Jan 09, 2023 at 09:38:41AM -0800, Eric Biggers wrote: > 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 ] > > I've applied this patchset for 6.3, but I'd still greatly appreciate reviews and > acks, especially on the last 4 patches, which touch files outside fs/verity/. > > (I applied it to > https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git/log/?h=fsverity for now, > but there might be a new git repo soon, as is being discussed elsewhere.) > > - Eric > The fs/verity patches look good to me, I've checked them but forgot to send RVB :( Haven't tested them yet though Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
On Mon, Jan 09, 2023 at 08:34:46PM +0100, Andrey Albershteyn wrote: > On Mon, Jan 09, 2023 at 09:38:41AM -0800, Eric Biggers wrote: > > 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 ] > > > > I've applied this patchset for 6.3, but I'd still greatly appreciate reviews and > > acks, especially on the last 4 patches, which touch files outside fs/verity/. > > > > (I applied it to > > https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git/log/?h=fsverity for now, > > but there might be a new git repo soon, as is being discussed elsewhere.) > > > > - Eric > > > > The fs/verity patches look good to me, I've checked them but forgot > to send RVB :( Haven't tested them yet though > > Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com> > Thanks Andrey! I added your Reviewed-by to patches 1-7 only, since you said "the fs/verity patches". Let me know if I can add it to patches 8-11 too. - Eric
On Fri, Dec 23, 2022 at 12:36:27PM -0800, Eric Biggers wrote: > 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 I'd still appreciate acks from the other ext4 and f2fs developers on these! - Eric
Hello: This series was applied to jaegeuk/f2fs.git (dev) by Eric Biggers <ebiggers@google.com>: On Fri, 23 Dec 2022 12:36:27 -0800 you 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. > > [...] Here is the summary with links: - [f2fs-dev,v2,01/11] fsverity: use unsigned long for level_start https://git.kernel.org/jaegeuk/f2fs/c/284d5db5f99e - [f2fs-dev,v2,02/11] fsverity: simplify Merkle tree readahead size calculation https://git.kernel.org/jaegeuk/f2fs/c/9098f36b739d - [f2fs-dev,v2,03/11] fsverity: store log2(digest_size) precomputed https://git.kernel.org/jaegeuk/f2fs/c/579a12f78d88 - [f2fs-dev,v2,04/11] fsverity: use EFBIG for file too large to enable verity https://git.kernel.org/jaegeuk/f2fs/c/55eed69cc8fd - [f2fs-dev,v2,05/11] fsverity: replace fsverity_hash_page() with fsverity_hash_block() https://git.kernel.org/jaegeuk/f2fs/c/f45555bf23cf - [f2fs-dev,v2,06/11] fsverity: support verification with tree block size < PAGE_SIZE https://git.kernel.org/jaegeuk/f2fs/c/5306892a50bf - [f2fs-dev,v2,07/11] fsverity: support enabling with tree block size < PAGE_SIZE https://git.kernel.org/jaegeuk/f2fs/c/56124d6c87fd - [f2fs-dev,v2,08/11] ext4: simplify ext4_readpage_limit() https://git.kernel.org/jaegeuk/f2fs/c/5e122148a3d5 - [f2fs-dev,v2,09/11] f2fs: simplify f2fs_readpage_limit() https://git.kernel.org/jaegeuk/f2fs/c/feb0576a361a - [f2fs-dev,v2,10/11] fs/buffer.c: support fsverity in block_read_full_folio() https://git.kernel.org/jaegeuk/f2fs/c/4fa512ce7051 - [f2fs-dev,v2,11/11] ext4: allow verity with fs block size < PAGE_SIZE https://git.kernel.org/jaegeuk/f2fs/c/db85d14dc5c5 You are awesome, thank you!
On Tue, Feb 28, 2023 at 01:01:54AM +0000, patchwork-bot+f2fs@kernel.org wrote: > Hello: > > This series was applied to jaegeuk/f2fs.git (dev) > by Eric Biggers <ebiggers@google.com>: > > On Fri, 23 Dec 2022 12:36:27 -0800 you 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. > > > > [...] > > Here is the summary with links: > - [f2fs-dev,v2,01/11] fsverity: use unsigned long for level_start > https://git.kernel.org/jaegeuk/f2fs/c/284d5db5f99e > - [f2fs-dev,v2,02/11] fsverity: simplify Merkle tree readahead size calculation > https://git.kernel.org/jaegeuk/f2fs/c/9098f36b739d > - [f2fs-dev,v2,03/11] fsverity: store log2(digest_size) precomputed > https://git.kernel.org/jaegeuk/f2fs/c/579a12f78d88 > - [f2fs-dev,v2,04/11] fsverity: use EFBIG for file too large to enable verity > https://git.kernel.org/jaegeuk/f2fs/c/55eed69cc8fd > - [f2fs-dev,v2,05/11] fsverity: replace fsverity_hash_page() with fsverity_hash_block() > https://git.kernel.org/jaegeuk/f2fs/c/f45555bf23cf > - [f2fs-dev,v2,06/11] fsverity: support verification with tree block size < PAGE_SIZE > https://git.kernel.org/jaegeuk/f2fs/c/5306892a50bf > - [f2fs-dev,v2,07/11] fsverity: support enabling with tree block size < PAGE_SIZE > https://git.kernel.org/jaegeuk/f2fs/c/56124d6c87fd > - [f2fs-dev,v2,08/11] ext4: simplify ext4_readpage_limit() > https://git.kernel.org/jaegeuk/f2fs/c/5e122148a3d5 > - [f2fs-dev,v2,09/11] f2fs: simplify f2fs_readpage_limit() > https://git.kernel.org/jaegeuk/f2fs/c/feb0576a361a > - [f2fs-dev,v2,10/11] fs/buffer.c: support fsverity in block_read_full_folio() > https://git.kernel.org/jaegeuk/f2fs/c/4fa512ce7051 > - [f2fs-dev,v2,11/11] ext4: allow verity with fs block size < PAGE_SIZE > https://git.kernel.org/jaegeuk/f2fs/c/db85d14dc5c5 > > You are awesome, thank you! > -- > Deet-doot-dot, I am a bot. > https://korg.docs.kernel.org/patchwork/pwbot.html > These commits reached the f2fs tree through mainline, not through being applied to the f2fs tree. So this email shouldn't have been sent. Jaegeuk, can you look into fixing the configuration of the f2fs patchwork bot to prevent this? - Eric
On 02/28, Eric Biggers wrote: > On Tue, Feb 28, 2023 at 01:01:54AM +0000, patchwork-bot+f2fs@kernel.org wrote: > > Hello: > > > > This series was applied to jaegeuk/f2fs.git (dev) > > by Eric Biggers <ebiggers@google.com>: > > > > On Fri, 23 Dec 2022 12:36:27 -0800 you 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. > > > > > > [...] > > > > Here is the summary with links: > > - [f2fs-dev,v2,01/11] fsverity: use unsigned long for level_start > > https://git.kernel.org/jaegeuk/f2fs/c/284d5db5f99e > > - [f2fs-dev,v2,02/11] fsverity: simplify Merkle tree readahead size calculation > > https://git.kernel.org/jaegeuk/f2fs/c/9098f36b739d > > - [f2fs-dev,v2,03/11] fsverity: store log2(digest_size) precomputed > > https://git.kernel.org/jaegeuk/f2fs/c/579a12f78d88 > > - [f2fs-dev,v2,04/11] fsverity: use EFBIG for file too large to enable verity > > https://git.kernel.org/jaegeuk/f2fs/c/55eed69cc8fd > > - [f2fs-dev,v2,05/11] fsverity: replace fsverity_hash_page() with fsverity_hash_block() > > https://git.kernel.org/jaegeuk/f2fs/c/f45555bf23cf > > - [f2fs-dev,v2,06/11] fsverity: support verification with tree block size < PAGE_SIZE > > https://git.kernel.org/jaegeuk/f2fs/c/5306892a50bf > > - [f2fs-dev,v2,07/11] fsverity: support enabling with tree block size < PAGE_SIZE > > https://git.kernel.org/jaegeuk/f2fs/c/56124d6c87fd > > - [f2fs-dev,v2,08/11] ext4: simplify ext4_readpage_limit() > > https://git.kernel.org/jaegeuk/f2fs/c/5e122148a3d5 > > - [f2fs-dev,v2,09/11] f2fs: simplify f2fs_readpage_limit() > > https://git.kernel.org/jaegeuk/f2fs/c/feb0576a361a > > - [f2fs-dev,v2,10/11] fs/buffer.c: support fsverity in block_read_full_folio() > > https://git.kernel.org/jaegeuk/f2fs/c/4fa512ce7051 > > - [f2fs-dev,v2,11/11] ext4: allow verity with fs block size < PAGE_SIZE > > https://git.kernel.org/jaegeuk/f2fs/c/db85d14dc5c5 > > > > You are awesome, thank you! > > -- > > Deet-doot-dot, I am a bot. > > https://korg.docs.kernel.org/patchwork/pwbot.html > > > > These commits reached the f2fs tree through mainline, not through being applied > to the f2fs tree. So this email shouldn't have been sent. Jaegeuk, can you > look into fixing the configuration of the f2fs patchwork bot to prevent this? Hmm, not sure how to fix that, since it seems patchwork bot reports this, once I pulled mainline into f2fs/dev branch. > > - Eric
[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(-)