[f2fs-dev][PATCH v2] f2fs: fsverity: Truncate cache pages if set verity failed

[f2fs-dev][PATCH v2] f2fs: fsverity: Truncate cache pages if set verity failed

[Yunlei He]

If file enable verity failed, should truncate anything wrote
past i_size, including cache pages.

Fixes: 95ae251fe828 ("f2fs: add fs-verity support")
Cc: <stable@vger.kernel.org> # v5.4+
Signed-off-by: Yunlei He <heyunlei@hihonor.com>
---
 fs/f2fs/verity.c | 47 +++++++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index 054ec852b5ea..3aa851affc46 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -158,33 +158,36 @@ static int f2fs_end_enable_verity(struct file *filp, const void *desc,
 		.size = cpu_to_le32(desc_size),
 		.pos = cpu_to_le64(desc_pos),
 	};
-	int err = 0;
+	int err;
 
-	if (desc != NULL) {
-		/* Succeeded; write the verity descriptor. */
-		err = pagecache_write(inode, desc, desc_size, desc_pos);
+	clear_inode_flag(inode, FI_VERITY_IN_PROGRESS);
+	if (!desc)
+		return 0;
 
-		/* Write all pages before clearing FI_VERITY_IN_PROGRESS. */
-		if (!err)
-			err = filemap_write_and_wait(inode->i_mapping);
-	}
+	/* Succeeded; write the verity descriptor. */
+	err = pagecache_write(inode, desc, desc_size, desc_pos);
+	if (err)
+		goto out;
 
-	/* If we failed, truncate anything we wrote past i_size. */
-	if (desc == NULL || err)
-		f2fs_truncate(inode);
+	err = filemap_write_and_wait(inode->i_mapping);
+	if (err)
+		goto out;
 
-	clear_inode_flag(inode, FI_VERITY_IN_PROGRESS);
+	err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_VERITY,
+			    F2FS_XATTR_NAME_VERITY, &dloc, sizeof(dloc),
+			    NULL, XATTR_CREATE);
+	if (err)
+		goto out;
 
-	if (desc != NULL && !err) {
-		err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_VERITY,
-				    F2FS_XATTR_NAME_VERITY, &dloc, sizeof(dloc),
-				    NULL, XATTR_CREATE);
-		if (!err) {
-			file_set_verity(inode);
-			f2fs_set_inode_flags(inode);
-			f2fs_mark_inode_dirty_sync(inode, true);
-		}
-	}
+	file_set_verity(inode);
+	f2fs_set_inode_flags(inode);
+	f2fs_mark_inode_dirty_sync(inode, true);
+
+	return 0;
+out:
+	/* If we failed, truncate anything we wrote past i_size. */
+	truncate_inode_pages(inode->i_mapping, inode->i_size);
+	f2fs_truncate(inode);
 	return err;
 }
 
-- 
2.17.1

Re: [PATCH v2] f2fs: fsverity: Truncate cache pages if set verity failed

[Eric Biggers]

On Mon, Mar 01, 2021 at 10:15:06PM +0800, Yunlei He wrote:
> If file enable verity failed, should truncate anything wrote
> past i_size, including cache pages.

The commit message and title need to be updated to properly describe the change.
They were okay for v1, but v2 makes additional changes.

> 
> Fixes: 95ae251fe828 ("f2fs: add fs-verity support")
> Cc: <stable@vger.kernel.org> # v5.4+
> Signed-off-by: Yunlei He <heyunlei@hihonor.com>
> ---
>  fs/f2fs/verity.c | 47 +++++++++++++++++++++++++----------------------
>  1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
> index 054ec852b5ea..3aa851affc46 100644
> --- a/fs/f2fs/verity.c
> +++ b/fs/f2fs/verity.c
> @@ -158,33 +158,36 @@ static int f2fs_end_enable_verity(struct file *filp, const void *desc,
>  		.size = cpu_to_le32(desc_size),
>  		.pos = cpu_to_le64(desc_pos),
>  	};
> -	int err = 0;
> +	int err;
>  
> -	if (desc != NULL) {
> -		/* Succeeded; write the verity descriptor. */
> -		err = pagecache_write(inode, desc, desc_size, desc_pos);
> +	clear_inode_flag(inode, FI_VERITY_IN_PROGRESS);
> +	if (!desc)
> +		return 0;

This isn't correct.  If desc == NULL (indicating that enabling verity failed),
it's still necessary to truncate the stuff past i_size.

>  
> -		/* Write all pages before clearing FI_VERITY_IN_PROGRESS. */
> -		if (!err)
> -			err = filemap_write_and_wait(inode->i_mapping);

As the comment above (which you deleted) says, all pages need to be written
before clearing FI_VERITY_IN_PROGRESS.

- Eric