PING
brookxu wrote on 2020/3/3 16:51:
> From: Chunguang Xu <brookxu@xxxxxxxxxxx>
>
> In the scene of deleting a file, the physical block information in the
> extent will be cleared to 0, the buffer_head contains these extents is
> marked as dirty, and then managed by jbd2, which will clear the
> buffer_head's dirty flag by clear_buffer_dirty. However, when the entire
> extent block is deleted, it is revoked from the jbd2, but the extents
> block is not redirtied.
>
> Not quite reasonable here, for the following concerns:
>
> 1. This has the risk of information leakage and leads to an interesting
> phenomenon that deleting the entire file is no more secure than truncate
> to 1 byte, because the whole extents physical block clear to zero in cache
> will never written back as the page is not redirtied.
>
> 2. For large files, the number of index block is usually very small.
> Ignoring index pages not get much more benefit in IO performance. But if
> we remark the page as dirty, the page is then written back by the system
> writeback mechanism asynchronously with little performance impact. As a
> result, the risk of information leakage can be avoided. At least not wrose
> than truncate file length to 1 byte
>
> Therefore, when the index block is released, we need to remark its page
> as dirty, so that the index block on the disk will be updated and the
> data is more security.
>
> Signed-off-by: Chunguang Xu <brookxu@xxxxxxxxxxx>
> ---
> fs/ext4/ext4.h | 1 +
> fs/ext4/ext4_jbd2.c | 8 ++++++++
> fs/ext4/extents.c | 3 ++-
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 61b37a0..f9a4d97 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -644,6 +644,7 @@ enum {
> #define EXT4_FREE_BLOCKS_NOFREE_FIRST_CLUSTER 0x0010
> #define EXT4_FREE_BLOCKS_NOFREE_LAST_CLUSTER 0x0020
> #define EXT4_FREE_BLOCKS_RERESERVE_CLUSTER 0x0040
> +#define EXT4_FREE_BLOCKS_METADATA_INDEX 0x0080
>
> /*
> * ioctl commands
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index 1f53d64..7974c62 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -275,7 +275,15 @@ int __ext4_forget(const char *where, unsigned int line, handle_t *handle,
> ext4_set_errno(inode->i_sb, -err);
> __ext4_abort(inode->i_sb, where, line,
> "error %d when attempting revoke", err);
> + } else {
> + /*
> + * we dirtied index block to ensure that related changes to
> + * the index block will be stored to disk.
> + */
> + if (is_metadata & EXT4_FREE_BLOCKS_METADATA_INDEX)
> + mark_buffer_dirty(bh);
> }
> +
> BUFFER_TRACE(bh, "exit");
> return err;
> }
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 954013d..2ee0df0 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2431,7 +2431,8 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
> trace_ext4_ext_rm_idx(inode, leaf);
>
> ext4_free_blocks(handle, inode, NULL, leaf, 1,
> - EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
> + EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET |
> + EXT4_FREE_BLOCKS_METADATA_INDEX);
>
> while (--depth >= 0) {
> if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr))
