diff mbox series

[v3,09/28] fsverity: pass log_blocksize to end_enable_verity()

Message ID 20231006184922.252188-10-aalbersh@redhat.com (mailing list archive)
State Superseded
Headers show
Series fs-verity support for XFS | expand

Commit Message

Andrey Albershteyn Oct. 6, 2023, 6:49 p.m. UTC
XFS will need to know log_blocksize to remove the tree in case of an
error. The size is needed to calculate offsets of particular Merkle
tree blocks.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---
 fs/btrfs/verity.c        | 4 +++-
 fs/ext4/verity.c         | 3 ++-
 fs/f2fs/verity.c         | 3 ++-
 fs/verity/enable.c       | 6 ++++--
 include/linux/fsverity.h | 4 +++-
 5 files changed, 14 insertions(+), 6 deletions(-)

Comments

Eric Biggers Oct. 11, 2023, 3:19 a.m. UTC | #1
On Fri, Oct 06, 2023 at 08:49:03PM +0200, Andrey Albershteyn wrote:
> diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
> index 252b2668894c..cac012d4c86a 100644
> --- a/include/linux/fsverity.h
> +++ b/include/linux/fsverity.h
> @@ -51,6 +51,7 @@ struct fsverity_operations {
>  	 * @desc: the verity descriptor to write, or NULL on failure
>  	 * @desc_size: size of verity descriptor, or 0 on failure
>  	 * @merkle_tree_size: total bytes the Merkle tree took up
> +	 * @log_blocksize: log size of the Merkle tree block
>  	 *
>  	 * If desc == NULL, then enabling verity failed and the filesystem only
>  	 * must do any necessary cleanups.  Else, it must also store the given
> @@ -65,7 +66,8 @@ struct fsverity_operations {
>  	 * Return: 0 on success, -errno on failure
>  	 */
>  	int (*end_enable_verity)(struct file *filp, const void *desc,
> -				 size_t desc_size, u64 merkle_tree_size);
> +				 size_t desc_size, u64 merkle_tree_size,
> +				 u8 log_blocksize);

Maybe just pass the block_size itself instead of log2(block_size)?

- Eric
Andrey Albershteyn Oct. 11, 2023, 11:17 a.m. UTC | #2
On 2023-10-10 20:19:06, Eric Biggers wrote:
> On Fri, Oct 06, 2023 at 08:49:03PM +0200, Andrey Albershteyn wrote:
> > diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
> > index 252b2668894c..cac012d4c86a 100644
> > --- a/include/linux/fsverity.h
> > +++ b/include/linux/fsverity.h
> > @@ -51,6 +51,7 @@ struct fsverity_operations {
> >  	 * @desc: the verity descriptor to write, or NULL on failure
> >  	 * @desc_size: size of verity descriptor, or 0 on failure
> >  	 * @merkle_tree_size: total bytes the Merkle tree took up
> > +	 * @log_blocksize: log size of the Merkle tree block
> >  	 *
> >  	 * If desc == NULL, then enabling verity failed and the filesystem only
> >  	 * must do any necessary cleanups.  Else, it must also store the given
> > @@ -65,7 +66,8 @@ struct fsverity_operations {
> >  	 * Return: 0 on success, -errno on failure
> >  	 */
> >  	int (*end_enable_verity)(struct file *filp, const void *desc,
> > -				 size_t desc_size, u64 merkle_tree_size);
> > +				 size_t desc_size, u64 merkle_tree_size,
> > +				 u8 log_blocksize);
> 
> Maybe just pass the block_size itself instead of log2(block_size)?

XFS will still do `index << log2(block_size)` to get block's offset.
So, not sure if there's any difference.
Eric Biggers Oct. 12, 2023, 7:34 a.m. UTC | #3
On Wed, Oct 11, 2023 at 01:17:36PM +0200, Andrey Albershteyn wrote:
> On 2023-10-10 20:19:06, Eric Biggers wrote:
> > On Fri, Oct 06, 2023 at 08:49:03PM +0200, Andrey Albershteyn wrote:
> > > diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
> > > index 252b2668894c..cac012d4c86a 100644
> > > --- a/include/linux/fsverity.h
> > > +++ b/include/linux/fsverity.h
> > > @@ -51,6 +51,7 @@ struct fsverity_operations {
> > >  	 * @desc: the verity descriptor to write, or NULL on failure
> > >  	 * @desc_size: size of verity descriptor, or 0 on failure
> > >  	 * @merkle_tree_size: total bytes the Merkle tree took up
> > > +	 * @log_blocksize: log size of the Merkle tree block
> > >  	 *
> > >  	 * If desc == NULL, then enabling verity failed and the filesystem only
> > >  	 * must do any necessary cleanups.  Else, it must also store the given
> > > @@ -65,7 +66,8 @@ struct fsverity_operations {
> > >  	 * Return: 0 on success, -errno on failure
> > >  	 */
> > >  	int (*end_enable_verity)(struct file *filp, const void *desc,
> > > -				 size_t desc_size, u64 merkle_tree_size);
> > > +				 size_t desc_size, u64 merkle_tree_size,
> > > +				 u8 log_blocksize);
> > 
> > Maybe just pass the block_size itself instead of log2(block_size)?
> 
> XFS will still do `index << log2(block_size)` to get block's offset.
> So, not sure if there's any difference.

It's only used in the following:

	offset = 0;
	for (index = 1; offset < merkle_tree_size; index++) {
		xfs_fsverity_merkle_key_to_disk(&name, offset);
		args.name = (const uint8_t *)&name.merkleoff;
		args.attr_filter = XFS_ATTR_VERITY;
		error = xfs_attr_set(&args);
		offset = index << log_blocksize;
	}

... which can be the following instead:

	for (offset = 0; offset < merkle_tree_size; offset += block_size) {
		xfs_fsverity_merkle_key_to_disk(&name, offset);
		args.name = (const uint8_t *)&name.merkleoff;
		args.attr_filter = XFS_ATTR_VERITY;
		error = xfs_attr_set(&args);
	}
diff mbox series

Patch

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index b39199b57a69..2b34796f68d3 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -621,6 +621,7 @@  static int btrfs_begin_enable_verity(struct file *filp)
  * @desc:              verity descriptor to write out (NULL in error conditions)
  * @desc_size:         size of the verity descriptor (variable with signatures)
  * @merkle_tree_size:  size of the merkle tree in bytes
+ * @log_blocksize:     log size of the Merkle tree block
  *
  * If desc is null, then VFS is signaling an error occurred during verity
  * enable, and we should try to rollback. Otherwise, attempt to finish verity.
@@ -628,7 +629,8 @@  static int btrfs_begin_enable_verity(struct file *filp)
  * Returns 0 on success, negative error code on error.
  */
 static int btrfs_end_enable_verity(struct file *filp, const void *desc,
-				   size_t desc_size, u64 merkle_tree_size)
+				   size_t desc_size, u64 merkle_tree_size,
+				   u8 log_blocksize)
 {
 	struct btrfs_inode *inode = BTRFS_I(file_inode(filp));
 	int ret = 0;
diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index 4eb77cefdbe1..4e2f01f048c0 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -189,7 +189,8 @@  static int ext4_write_verity_descriptor(struct inode *inode, const void *desc,
 }
 
 static int ext4_end_enable_verity(struct file *filp, const void *desc,
-				  size_t desc_size, u64 merkle_tree_size)
+				  size_t desc_size, u64 merkle_tree_size,
+				  u8 log_blocksize)
 {
 	struct inode *inode = file_inode(filp);
 	const int credits = 2; /* superblock and inode for ext4_orphan_del() */
diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index bb354ab8ca5a..601ab9f0c024 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -144,7 +144,8 @@  static int f2fs_begin_enable_verity(struct file *filp)
 }
 
 static int f2fs_end_enable_verity(struct file *filp, const void *desc,
-				  size_t desc_size, u64 merkle_tree_size)
+				  size_t desc_size, u64 merkle_tree_size,
+				  u8 log_blocksize)
 {
 	struct inode *inode = file_inode(filp);
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
diff --git a/fs/verity/enable.c b/fs/verity/enable.c
index c284f46d1b53..c87cab796f0b 100644
--- a/fs/verity/enable.c
+++ b/fs/verity/enable.c
@@ -274,7 +274,8 @@  static int enable_verity(struct file *filp,
 	 * Serialized with ->begin_enable_verity() by the inode lock.
 	 */
 	inode_lock(inode);
-	err = vops->end_enable_verity(filp, desc, desc_size, params.tree_size);
+	err = vops->end_enable_verity(filp, desc, desc_size, params.tree_size,
+				      desc->log_blocksize);
 	inode_unlock(inode);
 	if (err) {
 		fsverity_err(inode, "%ps() failed with err %d",
@@ -300,7 +301,8 @@  static int enable_verity(struct file *filp,
 
 rollback:
 	inode_lock(inode);
-	(void)vops->end_enable_verity(filp, NULL, 0, params.tree_size);
+	(void)vops->end_enable_verity(filp, NULL, 0, params.tree_size,
+				      desc->log_blocksize);
 	inode_unlock(inode);
 	goto out;
 }
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index 252b2668894c..cac012d4c86a 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -51,6 +51,7 @@  struct fsverity_operations {
 	 * @desc: the verity descriptor to write, or NULL on failure
 	 * @desc_size: size of verity descriptor, or 0 on failure
 	 * @merkle_tree_size: total bytes the Merkle tree took up
+	 * @log_blocksize: log size of the Merkle tree block
 	 *
 	 * If desc == NULL, then enabling verity failed and the filesystem only
 	 * must do any necessary cleanups.  Else, it must also store the given
@@ -65,7 +66,8 @@  struct fsverity_operations {
 	 * Return: 0 on success, -errno on failure
 	 */
 	int (*end_enable_verity)(struct file *filp, const void *desc,
-				 size_t desc_size, u64 merkle_tree_size);
+				 size_t desc_size, u64 merkle_tree_size,
+				 u8 log_blocksize);
 
 	/**
 	 * Get the verity descriptor of the given inode.