| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 4 Aug 2017 11:15:49 +0800
From: Chao Yu <yuchao0@...wei.com>
To: Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <chao@...nel.org>
CC: <linux-f2fs-devel@...ts.sourceforge.net>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] f2fs-tools: support inode checksum
Hi Jaegeuk,
On 2017/8/4 9:38, Jaegeuk Kim wrote:
> Hi Chao,
>
> It seems three is missing case when verifying the checksum, if the page is
> got from cache with some updates. We need to verify it when actually reading
> the node block.
Agreed.
>
> Let me modify your patch like this. Is that okay for you?
Looks good to me, thank you. ;)
Thanks,
>
> Thanks,
>
> ---
> fs/f2fs/f2fs.h | 3 ++-
> fs/f2fs/inode.c | 62 +++++++++++++++++++++++++++++--------------------------
> fs/f2fs/node.c | 7 ++++++-
> fs/f2fs/segment.c | 2 +-
> 4 files changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 5a078cd6f68d..44e46a31509b 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2362,7 +2362,8 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
> * inode.c
> */
> void f2fs_set_inode_flags(struct inode *inode);
> -void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct f2fs_node *node);
> +bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page);
> +void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page);
> struct inode *f2fs_iget(struct super_block *sb, unsigned long ino);
> struct inode *f2fs_iget_retry(struct super_block *sb, unsigned long ino);
> int try_to_free_nats(struct f2fs_sb_info *sbi, int nr_shrink);
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index 05c8aeb0101e..b4c401d456e7 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -108,9 +108,26 @@ static void __recover_inline_status(struct inode *inode, struct page *ipage)
> return;
> }
>
> -static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi,
> - struct f2fs_node *node)
> +static bool f2fs_enable_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
> {
> + struct f2fs_inode *ri = &F2FS_NODE(page)->i;
> + int extra_isize = le32_to_cpu(ri->i_extra_isize);
> +
> + if (!f2fs_sb_has_inode_chksum(sbi->sb))
> + return false;
> +
> + if (!RAW_IS_INODE(F2FS_NODE(page)) || !(ri->i_inline & F2FS_EXTRA_ATTR))
> + return false;
> +
> + if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
> + return false;
> +
> + return true;
> +}
> +
> +static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
> +{
> + struct f2fs_node *node = F2FS_NODE(page);
> struct f2fs_inode *ri = &node->i;
> __le32 ino = node->footer.ino;
> __le32 gen = ri->i_generation;
> @@ -131,39 +148,34 @@ static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi,
> return chksum;
> }
>
> -static bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi,
> - struct f2fs_node *node, struct f2fs_inode_info *fi)
> +bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page)
> {
> - struct f2fs_inode *ri = &node->i;
> + struct f2fs_inode *ri;
> __u32 provided, calculated;
>
> - if (!f2fs_sb_has_inode_chksum(sbi->sb))
> - return true;
> -
> - if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_inode_checksum))
> + if (!f2fs_enable_inode_chksum(sbi, page))
> return true;
>
> + ri = &F2FS_NODE(page)->i;
> provided = le32_to_cpu(ri->i_inode_checksum);
> - calculated = f2fs_inode_chksum(sbi, node);
> + calculated = f2fs_inode_chksum(sbi, page);
> +
> + if (provided != calculated)
> + f2fs_msg(sbi->sb, KERN_WARNING,
> + "checksum invalid, ino = %x, %x vs. %x",
> + ino_of_node(page), provided, calculated);
>
> return provided == calculated;
> }
>
> -void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct f2fs_node *node)
> +void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
> {
> - struct f2fs_inode *ri = &node->i;
> - int extra_isize = le32_to_cpu(ri->i_extra_isize);
> -
> - if (!f2fs_sb_has_inode_chksum(sbi->sb))
> - return;
> -
> - if (!RAW_IS_INODE(node) || !(ri->i_inline & F2FS_EXTRA_ATTR))
> - return;
> + struct f2fs_inode *ri = &F2FS_NODE(page)->i;
>
> - if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
> + if (!f2fs_enable_inode_chksum(sbi, page))
> return;
>
> - ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, node));
> + ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, page));
> }
>
> static int do_read_inode(struct inode *inode)
> @@ -219,14 +231,6 @@ static int do_read_inode(struct inode *inode)
> fi->i_extra_isize = f2fs_has_extra_attr(inode) ?
> le16_to_cpu(ri->i_extra_isize) : 0;
>
> - if (!f2fs_inode_chksum_verify(sbi, F2FS_NODE(node_page), fi)) {
> - f2fs_msg(sbi->sb, KERN_WARNING,
> - "checksum invalid, ino:%lu, on-disk:%u",
> - inode->i_ino, le32_to_cpu(ri->i_inode_checksum));
> - f2fs_put_page(node_page, 1);
> - return -EBADMSG;
> - }
> -
> /* check data exist */
> if (f2fs_has_inline_data(inode) && !f2fs_exist_data(inode))
> __recover_inline_status(inode, node_page);
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index b08f0d9bd86f..9168c304fd58 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1171,6 +1171,11 @@ static struct page *__get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid,
> err = -EIO;
> goto out_err;
> }
> +
> + if (!f2fs_inode_chksum_verify(sbi, page)) {
> + err = -EBADMSG;
> + goto out_err;
> + }
> page_hit:
> if(unlikely(nid != nid_of_node(page))) {
> f2fs_msg(sbi->sb, KERN_WARNING, "inconsistent node block, "
> @@ -2279,7 +2284,7 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
> i_projid))
> dst->i_projid = src->i_projid;
>
> - f2fs_inode_chksum_set(sbi, F2FS_NODE(ipage));
> + f2fs_inode_chksum_set(sbi, ipage);
> }
>
> new_ni = old_ni;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 151968ecc694..d9f4497890d7 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2222,7 +2222,7 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> if (page && IS_NODESEG(type)) {
> fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
>
> - f2fs_inode_chksum_set(sbi, F2FS_NODE(page));
> + f2fs_inode_chksum_set(sbi, page);
> }
>
> if (add_list) {
>
Powered by blists - more mailing lists