* [f2fs-dev] [PATCH 0/6] fs-verity fixups
@ 2019-08-11 21:35 Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 1/6] fs-verity: fix crash on read error in build_merkle_tree_level() Eric Biggers
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Eric Biggers @ 2019-08-11 21:35 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
A few fixes and cleanups for fs-verity.
If there are no objections, I'll fold these into the original patches.
Eric Biggers (6):
fs-verity: fix crash on read error in build_merkle_tree_level()
ext4: skip truncate when verity in progress in ->write_begin()
f2fs: skip truncate when verity in progress in ->write_begin()
ext4: remove ext4_bio_encrypted()
ext4: fix comment in ext4_end_enable_verity()
f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor()
fs/ext4/inode.c | 7 +++++--
fs/ext4/readpage.c | 9 ---------
fs/ext4/verity.c | 2 +-
fs/f2fs/data.c | 2 +-
fs/f2fs/verity.c | 2 +-
fs/verity/enable.c | 24 ++++++++++++++++--------
6 files changed, 24 insertions(+), 22 deletions(-)
--
2.22.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [f2fs-dev] [PATCH 1/6] fs-verity: fix crash on read error in build_merkle_tree_level()
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
@ 2019-08-11 21:35 ` Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 2/6] ext4: skip truncate when verity in progress in ->write_begin() Eric Biggers
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2019-08-11 21:35 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
From: Eric Biggers <ebiggers@google.com>
In build_merkle_tree_level(), use a separate fsverity_err() statement
when failing to read a data page, rather than incorrectly reusing the
one for failure to read a Merkle tree page and accessing level_start[]
out of bounds due to 'params->level_start[level - 1]' when level == 0.
Fixes: 248676649d53 ("fs-verity: implement FS_IOC_ENABLE_VERITY ioctl")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/verity/enable.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/fs/verity/enable.c b/fs/verity/enable.c
index 3371d51563962..eabc6ac199064 100644
--- a/fs/verity/enable.c
+++ b/fs/verity/enable.c
@@ -43,19 +43,27 @@ static int build_merkle_tree_level(struct inode *inode, unsigned int level,
pr_debug("Hashing block %llu of %llu for level %u\n",
i + 1, num_blocks_to_hash, level);
- if (level == 0)
+ if (level == 0) {
/* Leaf: hashing a data block */
src_page = read_mapping_page(inode->i_mapping, i, NULL);
- else
+ if (IS_ERR(src_page)) {
+ err = PTR_ERR(src_page);
+ fsverity_err(inode,
+ "Error %d reading data page %llu",
+ err, i);
+ return err;
+ }
+ } else {
/* Non-leaf: hashing hash block from level below */
src_page = vops->read_merkle_tree_page(inode,
params->level_start[level - 1] + i);
- if (IS_ERR(src_page)) {
- err = PTR_ERR(src_page);
- fsverity_err(inode,
- "Error %d reading Merkle tree page %llu",
- err, params->level_start[level - 1] + i);
- return err;
+ if (IS_ERR(src_page)) {
+ err = PTR_ERR(src_page);
+ fsverity_err(inode,
+ "Error %d reading Merkle tree page %llu",
+ err, params->level_start[level - 1] + i);
+ return err;
+ }
}
err = fsverity_hash_page(params, inode, req, src_page,
--
2.22.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [f2fs-dev] [PATCH 2/6] ext4: skip truncate when verity in progress in ->write_begin()
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 1/6] fs-verity: fix crash on read error in build_merkle_tree_level() Eric Biggers
@ 2019-08-11 21:35 ` Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 3/6] f2fs: " Eric Biggers
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2019-08-11 21:35 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
From: Eric Biggers <ebiggers@google.com>
When an error (e.g. ENOSPC) occurs during ext4_write_begin() when called
from ext4_write_merkle_tree_block(), skip truncating the file. i_size
is not meaningful in this case, and the truncation is handled by
ext4_end_enable_verity() instead. Also, this was triggering the
WARN_ON(!inode_is_locked(inode)) in ext4_truncate().
Fixes: ea54d7e4c0f8 ("ext4: add basic fs-verity support")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/ext4/inode.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b2c8d09acf652..cf0fce1173a4c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1340,6 +1340,9 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping,
}
if (ret) {
+ bool extended = (pos + len > inode->i_size) &&
+ !ext4_verity_in_progress(inode);
+
unlock_page(page);
/*
* __block_write_begin may have instantiated a few blocks
@@ -1349,11 +1352,11 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping,
* Add inode to orphan list in case we crash before
* truncate finishes
*/
- if (pos + len > inode->i_size && ext4_can_truncate(inode))
+ if (extended && ext4_can_truncate(inode))
ext4_orphan_add(handle, inode);
ext4_journal_stop(handle);
- if (pos + len > inode->i_size) {
+ if (extended) {
ext4_truncate_failed_write(inode);
/*
* If truncate failed early the inode might
--
2.22.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin()
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 1/6] fs-verity: fix crash on read error in build_merkle_tree_level() Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 2/6] ext4: skip truncate when verity in progress in ->write_begin() Eric Biggers
@ 2019-08-11 21:35 ` Eric Biggers
2019-08-12 12:25 ` Chao Yu
2019-08-11 21:35 ` [f2fs-dev] [PATCH 4/6] ext4: remove ext4_bio_encrypted() Eric Biggers
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2019-08-11 21:35 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
From: Eric Biggers <ebiggers@google.com>
When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called
from f2fs_write_merkle_tree_block(), skip truncating the file. i_size
is not meaningful in this case, and the truncation is handled by
f2fs_end_enable_verity() instead.
Fixes: 60d7bf0f790f ("f2fs: add fs-verity support")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/f2fs/data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 3f525f8a3a5fa..00b03fb87bd9b 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
struct inode *inode = mapping->host;
loff_t i_size = i_size_read(inode);
- if (to > i_size) {
+ if (to > i_size && !f2fs_verity_in_progress(inode)) {
down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
down_write(&F2FS_I(inode)->i_mmap_sem);
--
2.22.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [f2fs-dev] [PATCH 4/6] ext4: remove ext4_bio_encrypted()
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
` (2 preceding siblings ...)
2019-08-11 21:35 ` [f2fs-dev] [PATCH 3/6] f2fs: " Eric Biggers
@ 2019-08-11 21:35 ` Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 5/6] ext4: fix comment in ext4_end_enable_verity() Eric Biggers
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2019-08-11 21:35 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
From: Eric Biggers <ebiggers@google.com>
ext4_bio_encrypted() is unused following commit 4e47a0d40dac
("ext4: add fs-verity read support"), so remove it.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/ext4/readpage.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index ec8aeab3af65a..a30b203fa461c 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -52,15 +52,6 @@
static struct kmem_cache *bio_post_read_ctx_cache;
static mempool_t *bio_post_read_ctx_pool;
-static inline bool ext4_bio_encrypted(struct bio *bio)
-{
-#ifdef CONFIG_FS_ENCRYPTION
- return unlikely(bio->bi_private != NULL);
-#else
- return false;
-#endif
-}
-
/* postprocessing steps for read bios */
enum bio_post_read_step {
STEP_INITIAL = 0,
--
2.22.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [f2fs-dev] [PATCH 5/6] ext4: fix comment in ext4_end_enable_verity()
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
` (3 preceding siblings ...)
2019-08-11 21:35 ` [f2fs-dev] [PATCH 4/6] ext4: remove ext4_bio_encrypted() Eric Biggers
@ 2019-08-11 21:35 ` Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor() Eric Biggers
2019-08-18 20:23 ` [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
6 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2019-08-11 21:35 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
From: Eric Biggers <ebiggers@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/ext4/verity.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index bb0a3b8e6ea71..d0d8a9795dd62 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -196,7 +196,7 @@ static int ext4_end_enable_verity(struct file *filp, const void *desc,
size_t desc_size, u64 merkle_tree_size)
{
struct inode *inode = file_inode(filp);
- const int credits = 2; /* superblock and inode for ext4_orphan_add() */
+ const int credits = 2; /* superblock and inode for ext4_orphan_del() */
handle_t *handle;
int err = 0;
int err2;
--
2.22.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [f2fs-dev] [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor()
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
` (4 preceding siblings ...)
2019-08-11 21:35 ` [f2fs-dev] [PATCH 5/6] ext4: fix comment in ext4_end_enable_verity() Eric Biggers
@ 2019-08-11 21:35 ` Eric Biggers
2019-08-12 12:26 ` Chao Yu
2019-08-18 20:23 ` [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
6 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2019-08-11 21:35 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
From: Eric Biggers <ebiggers@google.com>
EFSCORRUPTED is now defined in f2fs.h, so use it instead of EUCLEAN.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/f2fs/verity.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index 6bc3470d99d00..a401ef72bc821 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -210,7 +210,7 @@ static int f2fs_get_verity_descriptor(struct inode *inode, void *buf,
if (pos + size < pos || pos + size > inode->i_sb->s_maxbytes ||
pos < f2fs_verity_metadata_pos(inode) || size > INT_MAX) {
f2fs_warn(F2FS_I_SB(inode), "invalid verity xattr");
- return -EUCLEAN; /* EFSCORRUPTED */
+ return -EFSCORRUPTED;
}
if (buf_size) {
if (size > buf_size)
--
2.22.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin()
2019-08-11 21:35 ` [f2fs-dev] [PATCH 3/6] f2fs: " Eric Biggers
@ 2019-08-12 12:25 ` Chao Yu
2019-08-12 22:58 ` Eric Biggers
0 siblings, 1 reply; 13+ messages in thread
From: Chao Yu @ 2019-08-12 12:25 UTC (permalink / raw)
To: Eric Biggers, linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
Hi Eric,
On 2019/8/12 5:35, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called
> from f2fs_write_merkle_tree_block(), skip truncating the file. i_size
> is not meaningful in this case, and the truncation is handled by
> f2fs_end_enable_verity() instead.
>
> Fixes: 60d7bf0f790f ("f2fs: add fs-verity support")
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/f2fs/data.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 3f525f8a3a5fa..00b03fb87bd9b 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
> struct inode *inode = mapping->host;
> loff_t i_size = i_size_read(inode);
>
> - if (to > i_size) {
Maybe adding one single line comment to mention that it's redundant/unnecessary
truncation here is better, if I didn't misunderstand this.
Thanks,
> + if (to > i_size && !f2fs_verity_in_progress(inode)) {
> down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> down_write(&F2FS_I(inode)->i_mmap_sem);
>
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [f2fs-dev] [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor()
2019-08-11 21:35 ` [f2fs-dev] [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor() Eric Biggers
@ 2019-08-12 12:26 ` Chao Yu
0 siblings, 0 replies; 13+ messages in thread
From: Chao Yu @ 2019-08-12 12:26 UTC (permalink / raw)
To: Eric Biggers, linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
On 2019/8/12 5:35, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> EFSCORRUPTED is now defined in f2fs.h, so use it instead of EUCLEAN.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Chao Yu <yuchao0@huawei.com>
Thanks,
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin()
2019-08-12 12:25 ` Chao Yu
@ 2019-08-12 22:58 ` Eric Biggers
2019-08-13 2:40 ` Chao Yu
0 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2019-08-12 22:58 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel
Hi Chao,
On Mon, Aug 12, 2019 at 08:25:33PM +0800, Chao Yu wrote:
> Hi Eric,
>
> On 2019/8/12 5:35, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called
> > from f2fs_write_merkle_tree_block(), skip truncating the file. i_size
> > is not meaningful in this case, and the truncation is handled by
> > f2fs_end_enable_verity() instead.
> >
> > Fixes: 60d7bf0f790f ("f2fs: add fs-verity support")
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > fs/f2fs/data.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 3f525f8a3a5fa..00b03fb87bd9b 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
> > struct inode *inode = mapping->host;
> > loff_t i_size = i_size_read(inode);
> >
> > - if (to > i_size) {
>
> Maybe adding one single line comment to mention that it's redundant/unnecessary
> truncation here is better, if I didn't misunderstand this.
>
> Thanks,
>
> > + if (to > i_size && !f2fs_verity_in_progress(inode)) {
> > down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> > down_write(&F2FS_I(inode)->i_mmap_sem);
> >
Do you mean add a comment instead of the !f2fs_verity_in_progress() check, or in
addition to it? ->write_begin(), ->writepages(), and ->write_end() are all
supposed to ignore i_size when verity is in progress, so I don't think this
particular part should be different, even if technically it's still correct to
truncate twice. Also, ext4 needs this check in its ->write_begin() for locking
reasons; we should keep f2fs similar.
How about having both a comment and the check, like:
/* In the fs-verity case, f2fs_end_enable_verity() does the truncate */
if (to > i_size && !f2fs_verity_in_progress(inode)) {
- Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin()
2019-08-12 22:58 ` Eric Biggers
@ 2019-08-13 2:40 ` Chao Yu
2019-08-18 20:24 ` Eric Biggers
0 siblings, 1 reply; 13+ messages in thread
From: Chao Yu @ 2019-08-13 2:40 UTC (permalink / raw)
To: linux-fscrypt, linux-ext4, linux-f2fs-devel
Hi Eric,
On 2019/8/13 6:58, Eric Biggers wrote:
> Hi Chao,
>
> On Mon, Aug 12, 2019 at 08:25:33PM +0800, Chao Yu wrote:
>> Hi Eric,
>>
>> On 2019/8/12 5:35, Eric Biggers wrote:
>>> From: Eric Biggers <ebiggers@google.com>
>>>
>>> When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called
>>> from f2fs_write_merkle_tree_block(), skip truncating the file. i_size
>>> is not meaningful in this case, and the truncation is handled by
>>> f2fs_end_enable_verity() instead.
>>>
>>> Fixes: 60d7bf0f790f ("f2fs: add fs-verity support")
>>> Signed-off-by: Eric Biggers <ebiggers@google.com>
>>> ---
>>> fs/f2fs/data.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 3f525f8a3a5fa..00b03fb87bd9b 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
>>> struct inode *inode = mapping->host;
>>> loff_t i_size = i_size_read(inode);
>>>
>>> - if (to > i_size) {
>>
>> Maybe adding one single line comment to mention that it's redundant/unnecessary
>> truncation here is better, if I didn't misunderstand this.
>>
>> Thanks,
>>
>>> + if (to > i_size && !f2fs_verity_in_progress(inode)) {
>>> down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>> down_write(&F2FS_I(inode)->i_mmap_sem);
>>>
>
> Do you mean add a comment instead of the !f2fs_verity_in_progress() check, or in
> addition to it? ->write_begin(), ->writepages(), and ->write_end() are all
Sorry, I didn't make this very clear, I meant adding the comment in addition on
above change.
> supposed to ignore i_size when verity is in progress, so I don't think this
> particular part should be different, even if technically it's still correct to
> truncate twice. Also, ext4 needs this check in its ->write_begin() for locking
> reasons; we should keep f2fs similar.
Agreed.
>
> How about having both a comment and the check, like:
>
> /* In the fs-verity case, f2fs_end_enable_verity() does the truncate */
> if (to > i_size && !f2fs_verity_in_progress(inode)) {
The comment looks good to me. :)
Thanks,
>
> - Eric
> .
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [f2fs-dev] [PATCH 0/6] fs-verity fixups
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
` (5 preceding siblings ...)
2019-08-11 21:35 ` [f2fs-dev] [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor() Eric Biggers
@ 2019-08-18 20:23 ` Eric Biggers
6 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2019-08-18 20:23 UTC (permalink / raw)
To: linux-fscrypt; +Cc: linux-ext4, linux-f2fs-devel
On Sun, Aug 11, 2019 at 02:35:51PM -0700, Eric Biggers wrote:
> A few fixes and cleanups for fs-verity.
>
> If there are no objections, I'll fold these into the original patches.
>
> Eric Biggers (6):
> fs-verity: fix crash on read error in build_merkle_tree_level()
> ext4: skip truncate when verity in progress in ->write_begin()
> f2fs: skip truncate when verity in progress in ->write_begin()
> ext4: remove ext4_bio_encrypted()
> ext4: fix comment in ext4_end_enable_verity()
> f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor()
>
> fs/ext4/inode.c | 7 +++++--
> fs/ext4/readpage.c | 9 ---------
> fs/ext4/verity.c | 2 +-
> fs/f2fs/data.c | 2 +-
> fs/f2fs/verity.c | 2 +-
> fs/verity/enable.c | 24 ++++++++++++++++--------
> 6 files changed, 24 insertions(+), 22 deletions(-)
>
> --
> 2.22.0
>
I've gone ahead and folded in these fixes.
- Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin()
2019-08-13 2:40 ` Chao Yu
@ 2019-08-18 20:24 ` Eric Biggers
0 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2019-08-18 20:24 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel
On Tue, Aug 13, 2019 at 10:40:39AM +0800, Chao Yu wrote:
> Hi Eric,
>
> On 2019/8/13 6:58, Eric Biggers wrote:
> > Hi Chao,
> >
> > On Mon, Aug 12, 2019 at 08:25:33PM +0800, Chao Yu wrote:
> >> Hi Eric,
> >>
> >> On 2019/8/12 5:35, Eric Biggers wrote:
> >>> From: Eric Biggers <ebiggers@google.com>
> >>>
> >>> When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called
> >>> from f2fs_write_merkle_tree_block(), skip truncating the file. i_size
> >>> is not meaningful in this case, and the truncation is handled by
> >>> f2fs_end_enable_verity() instead.
> >>>
> >>> Fixes: 60d7bf0f790f ("f2fs: add fs-verity support")
> >>> Signed-off-by: Eric Biggers <ebiggers@google.com>
> >>> ---
> >>> fs/f2fs/data.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >>> index 3f525f8a3a5fa..00b03fb87bd9b 100644
> >>> --- a/fs/f2fs/data.c
> >>> +++ b/fs/f2fs/data.c
> >>> @@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
> >>> struct inode *inode = mapping->host;
> >>> loff_t i_size = i_size_read(inode);
> >>>
> >>> - if (to > i_size) {
> >>
> >> Maybe adding one single line comment to mention that it's redundant/unnecessary
> >> truncation here is better, if I didn't misunderstand this.
> >>
> >> Thanks,
> >>
> >>> + if (to > i_size && !f2fs_verity_in_progress(inode)) {
> >>> down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >>> down_write(&F2FS_I(inode)->i_mmap_sem);
> >>>
> >
> > Do you mean add a comment instead of the !f2fs_verity_in_progress() check, or in
> > addition to it? ->write_begin(), ->writepages(), and ->write_end() are all
>
> Sorry, I didn't make this very clear, I meant adding the comment in addition on
> above change.
>
> > supposed to ignore i_size when verity is in progress, so I don't think this
> > particular part should be different, even if technically it's still correct to
> > truncate twice. Also, ext4 needs this check in its ->write_begin() for locking
> > reasons; we should keep f2fs similar.
>
> Agreed.
>
> >
> > How about having both a comment and the check, like:
> >
> > /* In the fs-verity case, f2fs_end_enable_verity() does the truncate */
> > if (to > i_size && !f2fs_verity_in_progress(inode)) {
>
> The comment looks good to me. :)
>
> Thanks,
>
> >
> > - Eric
> > .
> >
>
Okay, this is what I applied:
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 3f525f8a3a5f..54cad80acb7d 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2476,7 +2476,8 @@ static void f2fs_write_failed(struct address_space *mapping, loff_t to)
struct inode *inode = mapping->host;
loff_t i_size = i_size_read(inode);
- if (to > i_size) {
+ /* In the fs-verity case, f2fs_end_enable_verity() does the truncate */
+ if (to > i_size && !f2fs_verity_in_progress(inode)) {
down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
down_write(&F2FS_I(inode)->i_mmap_sem);
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-18 20:24 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-11 21:35 [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 1/6] fs-verity: fix crash on read error in build_merkle_tree_level() Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 2/6] ext4: skip truncate when verity in progress in ->write_begin() Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 3/6] f2fs: " Eric Biggers
2019-08-12 12:25 ` Chao Yu
2019-08-12 22:58 ` Eric Biggers
2019-08-13 2:40 ` Chao Yu
2019-08-18 20:24 ` Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 4/6] ext4: remove ext4_bio_encrypted() Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 5/6] ext4: fix comment in ext4_end_enable_verity() Eric Biggers
2019-08-11 21:35 ` [f2fs-dev] [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor() Eric Biggers
2019-08-12 12:26 ` Chao Yu
2019-08-18 20:23 ` [f2fs-dev] [PATCH 0/6] fs-verity fixups Eric Biggers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).