Commit f053cf7a authored by Eric Biggers's avatar Eric Biggers Committed by Theodore Ts'o

ext4: fix error handling in ext4_end_enable_verity()

ext4 didn't properly clean up if verity failed to be enabled on a file:

- It left verity metadata (pages past EOF) in the page cache, which
  would be exposed to userspace if the file was later extended.

- It didn't truncate the verity metadata at all (either from cache or
  from disk) if an error occurred while setting the verity bit.

Fix these bugs by adding a call to truncate_inode_pages() and ensuring
that we truncate the verity metadata (both from cache and from disk) in
all error paths.  Also rework the code to cleanly separate the success
path from the error paths, which makes it much easier to understand.
Reported-by: default avatarYunlei He <heyunlei@hihonor.com>
Fixes: c93d8f88 ("ext4: add basic fs-verity support")
Cc: stable@vger.kernel.org # v5.4+
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20210302200420.137977-2-ebiggers@kernel.orgSigned-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent c915fb80
...@@ -201,55 +201,76 @@ static int ext4_end_enable_verity(struct file *filp, const void *desc, ...@@ -201,55 +201,76 @@ static int ext4_end_enable_verity(struct file *filp, const void *desc,
struct inode *inode = file_inode(filp); struct inode *inode = file_inode(filp);
const int credits = 2; /* superblock and inode for ext4_orphan_del() */ const int credits = 2; /* superblock and inode for ext4_orphan_del() */
handle_t *handle; handle_t *handle;
struct ext4_iloc iloc;
int err = 0; int err = 0;
int err2;
if (desc != NULL) { /*
/* Succeeded; write the verity descriptor. */ * If an error already occurred (which fs/verity/ signals by passing
* desc == NULL), then only clean-up is needed.
*/
if (desc == NULL)
goto cleanup;
/* Append the verity descriptor. */
err = ext4_write_verity_descriptor(inode, desc, desc_size, err = ext4_write_verity_descriptor(inode, desc, desc_size,
merkle_tree_size); merkle_tree_size);
if (err)
goto cleanup;
/* Write all pages before clearing VERITY_IN_PROGRESS. */ /*
if (!err) * Write all pages (both data and verity metadata). Note that this must
* happen before clearing EXT4_STATE_VERITY_IN_PROGRESS; otherwise pages
* beyond i_size won't be written properly. For crash consistency, this
* also must happen before the verity inode flag gets persisted.
*/
err = filemap_write_and_wait(inode->i_mapping); err = filemap_write_and_wait(inode->i_mapping);
} if (err)
goto cleanup;
/* If we failed, truncate anything we wrote past i_size. */
if (desc == NULL || err)
ext4_truncate(inode);
/* /*
* We must always clean up by clearing EXT4_STATE_VERITY_IN_PROGRESS and * Finally, set the verity inode flag and remove the inode from the
* deleting the inode from the orphan list, even if something failed. * orphan list (in a single transaction).
* If everything succeeded, we'll also set the verity bit in the same
* transaction.
*/ */
ext4_clear_inode_state(inode, EXT4_STATE_VERITY_IN_PROGRESS);
handle = ext4_journal_start(inode, EXT4_HT_INODE, credits); handle = ext4_journal_start(inode, EXT4_HT_INODE, credits);
if (IS_ERR(handle)) { if (IS_ERR(handle)) {
ext4_orphan_del(NULL, inode); err = PTR_ERR(handle);
return PTR_ERR(handle); goto cleanup;
} }
err2 = ext4_orphan_del(handle, inode); err = ext4_orphan_del(handle, inode);
if (err2) if (err)
goto out_stop; goto stop_and_cleanup;
if (desc != NULL && !err) {
struct ext4_iloc iloc;
err = ext4_reserve_inode_write(handle, inode, &iloc); err = ext4_reserve_inode_write(handle, inode, &iloc);
if (err) if (err)
goto out_stop; goto stop_and_cleanup;
ext4_set_inode_flag(inode, EXT4_INODE_VERITY); ext4_set_inode_flag(inode, EXT4_INODE_VERITY);
ext4_set_inode_flags(inode, false); ext4_set_inode_flags(inode, false);
err = ext4_mark_iloc_dirty(handle, inode, &iloc); err = ext4_mark_iloc_dirty(handle, inode, &iloc);
} if (err)
out_stop: goto stop_and_cleanup;
ext4_journal_stop(handle);
ext4_clear_inode_state(inode, EXT4_STATE_VERITY_IN_PROGRESS);
return 0;
stop_and_cleanup:
ext4_journal_stop(handle); ext4_journal_stop(handle);
return err ?: err2; cleanup:
/*
* Verity failed to be enabled, so clean up by truncating any verity
* metadata that was written beyond i_size (both from cache and from
* disk), removing the inode from the orphan list (if it wasn't done
* already), and clearing EXT4_STATE_VERITY_IN_PROGRESS.
*/
truncate_inode_pages(inode->i_mapping, inode->i_size);
ext4_truncate(inode);
ext4_orphan_del(NULL, inode);
ext4_clear_inode_state(inode, EXT4_STATE_VERITY_IN_PROGRESS);
return err;
} }
static int ext4_get_verity_descriptor_location(struct inode *inode, static int ext4_get_verity_descriptor_location(struct inode *inode,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment