From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: Chao Yu <chao.yu@linux.dev>,
linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: compress: do sanity check on cluster
Date: Thu, 12 Aug 2021 13:49:46 -0700 [thread overview]
Message-ID: <YRWJan4JTXHN3RFr@google.com> (raw)
In-Reply-To: <20210806000250.39728-1-chao@kernel.org>
On 08/06, Chao Yu wrote:
> This patch adds f2fs_sanity_check_cluster() to support doing
> sanity check on cluster of compressed file, it will be triggered
> from below two paths:
>
> - __f2fs_cluster_blocks()
> - f2fs_map_blocks(F2FS_GET_BLOCK_FIEMAP)
>
> And it can detect below three kind of cluster insanity status.
>
> C: COMPRESS_ADDR
> N: NULL_ADDR or NEW_ADDR
> V: valid blkaddr
> *: any value
>
> 1. [*|C|*|*]
> 2. [C|*|C|*]
> 3. [C|N|N|V]
>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> v2:
> - cover all map_block cases
> - give EFSCORRUPTED only when CHECK_FS is enabled for fiemap()
> fs/f2fs/compress.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/f2fs/data.c | 9 ++++++++
> fs/f2fs/f2fs.h | 1 +
> 3 files changed, 63 insertions(+)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 7dbfd6965b97..f25b32a6893a 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -898,6 +898,54 @@ static bool cluster_has_invalid_data(struct compress_ctx *cc)
> return false;
> }
>
> +bool f2fs_sanity_check_cluster(struct dnode_of_data *dn)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> + unsigned int cluster_size = F2FS_I(dn->inode)->i_cluster_size;
> + bool compressed = dn->data_blkaddr == COMPRESS_ADDR;
> + int cluster_end = 0;
> + int i;
> + char *reason = "";
> +
> + if (!compressed)
> + return false;
> +
> + /* [..., COMPR_ADDR, ...] */
> + if (dn->ofs_in_node % cluster_size) {
> + reason = "[*|C|*|*]";
> + goto out;
> + }
> +
> + for (i = 1; i < cluster_size; i++) {
> + block_t blkaddr = data_blkaddr(dn->inode, dn->node_page,
> + dn->ofs_in_node + i);
> +
> + /* [COMPR_ADDR, ..., COMPR_ADDR] */
> + if (blkaddr == COMPRESS_ADDR) {
> + reason = "[C|*|C|*]";
> + goto out;
> + }
> + if (compressed) {
> + if (!__is_valid_data_blkaddr(blkaddr)) {
> + if (!cluster_end)
> + cluster_end = i;
> + continue;
> + }
> + /* [COMPR_ADDR, NULL_ADDR or NEW_ADDR, valid_blkaddr] */
> + if (cluster_end) {
> + reason = "[C|N|N|V]";
> + goto out;
> + }
> + }
> + }
> + return false;
> +out:
> + f2fs_warn(sbi, "access invalid cluster, ino:%lu, nid:%u, ofs_in_node:%u, reason:%s",
> + dn->inode->i_ino, dn->nid, dn->ofs_in_node, reason);
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + return true;
> +}
> +
> static int __f2fs_cluster_blocks(struct inode *inode,
> unsigned int cluster_idx, bool compr)
> {
> @@ -915,6 +963,11 @@ static int __f2fs_cluster_blocks(struct inode *inode,
> goto fail;
> }
>
> + if (f2fs_sanity_check_cluster(&dn)) {
> + ret = -EFSCORRUPTED;
> + goto fail;
> + }
> +
> if (dn.data_blkaddr == COMPRESS_ADDR) {
> int i;
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index df5e8d8c654e..d4c9aeba0842 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1552,6 +1552,15 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
> map->m_flags |= F2FS_MAP_NEW;
> blkaddr = dn.data_blkaddr;
> } else {
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
I tried to remove ifdef. Please check f2fs/dev branch.
> + if (f2fs_compressed_file(inode) &&
> + f2fs_sanity_check_cluster(&dn) &&
> + (flag != F2FS_GET_BLOCK_FIEMAP ||
> + IS_ENABLED(CONFIG_F2FS_CHECK_FS))) {
> + err = -EFSCORRUPTED;
> + goto sync_out;
> + }
> +#endif
> if (flag == F2FS_GET_BLOCK_BMAP) {
> map->m_pblk = 0;
> goto sync_out;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index e97b4d8c5efc..3b368bcbc4d7 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4074,6 +4074,7 @@ void f2fs_end_read_compressed_page(struct page *page, bool failed,
> block_t blkaddr);
> bool f2fs_cluster_is_empty(struct compress_ctx *cc);
> bool f2fs_cluster_can_merge_page(struct compress_ctx *cc, pgoff_t index);
> +bool f2fs_sanity_check_cluster(struct dnode_of_data *dn);
> void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page);
> int f2fs_write_multi_pages(struct compress_ctx *cc,
> int *submitted,
> --
> 2.22.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, Chao Yu <chao.yu@linux.dev>
Subject: Re: [PATCH v2] f2fs: compress: do sanity check on cluster
Date: Thu, 12 Aug 2021 13:49:46 -0700 [thread overview]
Message-ID: <YRWJan4JTXHN3RFr@google.com> (raw)
In-Reply-To: <20210806000250.39728-1-chao@kernel.org>
On 08/06, Chao Yu wrote:
> This patch adds f2fs_sanity_check_cluster() to support doing
> sanity check on cluster of compressed file, it will be triggered
> from below two paths:
>
> - __f2fs_cluster_blocks()
> - f2fs_map_blocks(F2FS_GET_BLOCK_FIEMAP)
>
> And it can detect below three kind of cluster insanity status.
>
> C: COMPRESS_ADDR
> N: NULL_ADDR or NEW_ADDR
> V: valid blkaddr
> *: any value
>
> 1. [*|C|*|*]
> 2. [C|*|C|*]
> 3. [C|N|N|V]
>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> v2:
> - cover all map_block cases
> - give EFSCORRUPTED only when CHECK_FS is enabled for fiemap()
> fs/f2fs/compress.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/f2fs/data.c | 9 ++++++++
> fs/f2fs/f2fs.h | 1 +
> 3 files changed, 63 insertions(+)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 7dbfd6965b97..f25b32a6893a 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -898,6 +898,54 @@ static bool cluster_has_invalid_data(struct compress_ctx *cc)
> return false;
> }
>
> +bool f2fs_sanity_check_cluster(struct dnode_of_data *dn)
> +{
> + struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> + unsigned int cluster_size = F2FS_I(dn->inode)->i_cluster_size;
> + bool compressed = dn->data_blkaddr == COMPRESS_ADDR;
> + int cluster_end = 0;
> + int i;
> + char *reason = "";
> +
> + if (!compressed)
> + return false;
> +
> + /* [..., COMPR_ADDR, ...] */
> + if (dn->ofs_in_node % cluster_size) {
> + reason = "[*|C|*|*]";
> + goto out;
> + }
> +
> + for (i = 1; i < cluster_size; i++) {
> + block_t blkaddr = data_blkaddr(dn->inode, dn->node_page,
> + dn->ofs_in_node + i);
> +
> + /* [COMPR_ADDR, ..., COMPR_ADDR] */
> + if (blkaddr == COMPRESS_ADDR) {
> + reason = "[C|*|C|*]";
> + goto out;
> + }
> + if (compressed) {
> + if (!__is_valid_data_blkaddr(blkaddr)) {
> + if (!cluster_end)
> + cluster_end = i;
> + continue;
> + }
> + /* [COMPR_ADDR, NULL_ADDR or NEW_ADDR, valid_blkaddr] */
> + if (cluster_end) {
> + reason = "[C|N|N|V]";
> + goto out;
> + }
> + }
> + }
> + return false;
> +out:
> + f2fs_warn(sbi, "access invalid cluster, ino:%lu, nid:%u, ofs_in_node:%u, reason:%s",
> + dn->inode->i_ino, dn->nid, dn->ofs_in_node, reason);
> + set_sbi_flag(sbi, SBI_NEED_FSCK);
> + return true;
> +}
> +
> static int __f2fs_cluster_blocks(struct inode *inode,
> unsigned int cluster_idx, bool compr)
> {
> @@ -915,6 +963,11 @@ static int __f2fs_cluster_blocks(struct inode *inode,
> goto fail;
> }
>
> + if (f2fs_sanity_check_cluster(&dn)) {
> + ret = -EFSCORRUPTED;
> + goto fail;
> + }
> +
> if (dn.data_blkaddr == COMPRESS_ADDR) {
> int i;
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index df5e8d8c654e..d4c9aeba0842 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1552,6 +1552,15 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
> map->m_flags |= F2FS_MAP_NEW;
> blkaddr = dn.data_blkaddr;
> } else {
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
I tried to remove ifdef. Please check f2fs/dev branch.
> + if (f2fs_compressed_file(inode) &&
> + f2fs_sanity_check_cluster(&dn) &&
> + (flag != F2FS_GET_BLOCK_FIEMAP ||
> + IS_ENABLED(CONFIG_F2FS_CHECK_FS))) {
> + err = -EFSCORRUPTED;
> + goto sync_out;
> + }
> +#endif
> if (flag == F2FS_GET_BLOCK_BMAP) {
> map->m_pblk = 0;
> goto sync_out;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index e97b4d8c5efc..3b368bcbc4d7 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4074,6 +4074,7 @@ void f2fs_end_read_compressed_page(struct page *page, bool failed,
> block_t blkaddr);
> bool f2fs_cluster_is_empty(struct compress_ctx *cc);
> bool f2fs_cluster_can_merge_page(struct compress_ctx *cc, pgoff_t index);
> +bool f2fs_sanity_check_cluster(struct dnode_of_data *dn);
> void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page);
> int f2fs_write_multi_pages(struct compress_ctx *cc,
> int *submitted,
> --
> 2.22.1
next prev parent reply other threads:[~2021-08-12 20:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-06 0:02 [f2fs-dev] [PATCH v2] f2fs: compress: do sanity check on cluster Chao Yu
2021-08-06 0:02 ` Chao Yu
2021-08-12 20:49 ` Jaegeuk Kim [this message]
2021-08-12 20:49 ` Jaegeuk Kim
2021-08-13 1:08 ` [f2fs-dev] " Chao Yu
2021-08-13 1:08 ` Chao Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YRWJan4JTXHN3RFr@google.com \
--to=jaegeuk@kernel.org \
--cc=chao.yu@linux.dev \
--cc=chao@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.