Messages in this thread | | | Subject | Re: [PATCH] f2fs-tools: support inode checksum | From | Chao Yu <> | Date | Fri, 4 Aug 2017 11:15:49 +0800 |
| |
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) { >
| |