linux-ext4 - Re: [PATCH 4/4] fsverity: pass pos and size to ->write_merkle_tree

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Date:   Wed, 25 Jan 2023 10:12:30 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Andrey Albershteyn <aalbersh@...hat.com>
Cc:     linux-fscrypt@...r.kernel.org, linux-xfs@...r.kernel.org,
        linux-ext4@...r.kernel.org, Dave Chinner <david@...morbit.com>,
        linux-btrfs@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [PATCH 4/4] fsverity: pass pos and size to
 ->write_merkle_tree_block

[Added back Cc's.  Please use Reply-All instead of Reply!]

On Wed, Jan 25, 2023 at 01:22:27PM +0100, Andrey Albershteyn wrote:
> On Wed, Dec 14, 2022 at 02:43:04PM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@...gle.com>
> > 
> > fsverity_operations::write_merkle_tree_block is passed the index of the
> > block to write and the log base 2 of the block size.  However, all
> > implementations of it use these parameters only to calculate the
> > position and the size of the block, in bytes.
> > 
> > Therefore, make ->write_merkle_tree_block take 'pos' and 'size'
> > parameters instead of 'index' and 'log_blocksize'.
> 
> Hi Eric,
> 
> Thanks for the quick responses with changes to fs-verity!
> 
> With this patch shouldn't the read_merkle_tree_block() also change
> to [pos, size] args? I see that ext4 uses index to read the page at
> that index + file size. But I suppose that when Merkle tree block
> size will vary (e.g. 8k) it will require position + size.

Not yet.  It's actually read_merkle_tree_page(), not read_merkle_tree_block().
The callees want a page index, and pages always have size PAGE_SIZE.  So the
current function prototype is appropriate for the current design.

> In XFS as we store the page under the xattr with "pos" as a name
> we also need a "pos" while reading the page. So, currently XFS can
> use index << log2(PAGE_SHIFT) or will need to get also log_blocksize
> from descriptor.

But you definitely need to think about what changes should be made to allow XFS
to do the Merkle tree caching the way the other XFS developers want it to do.
There will be significantly more to that than potentially changing a function
prototype.  There's been some discussion of this on the "fs-verity support for
XFS" thread, but there's not a detailed proposal yet.

Note: you should store Merkle tree blocks in the xattrs instead of "pages", so
that the on-disk format isn't tied to the page size.

- Eric