linux-kernel - Re: [PATCH] f2fs: Fix recover when nid of non-inode dnode

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 09 Sep 2014 13:39:30 +0800
From:	Huang Ying <ying.huang@...el.com>
To:	Jaegeuk Kim <jaegeuk@...nel.org>
Cc:	Changman Lee <cm224.lee@...sung.com>,
	linux-f2fs-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, huang.ying.caritas@...il.com
Subject: Re: [PATCH] f2fs: Fix recover when nid of non-inode dnode < nid of
 inode

On Mon, 2014-09-08 at 22:23 -0700, Jaegeuk Kim wrote:
> Hi Huang,
> 
> On Mon, Sep 08, 2014 at 07:38:26PM +0800, Huang Ying wrote:
> > For fsync, if the nid of a non-inode dnode < nid of inode and the
> > inode is not checkpointed.  The non-inode dnode may be written before
> > inode.  So in find_fsync_dnodes, f2fs_iget will fail, cause the
> > recovery fail.
> > 
> > Usually, inode will be allocated before non-inode dnode, so the nid of
> > inode < nid of non-inode dnode.  But it is possible for the reverse.
> > For example, because of alloc_nid_failed.
> > 
> > This is fixed via ignoring non-inode dnode before inode dnode in
> > find_fsync_dnodes.
> > 
> > The patch was tested via allocating nid reversely via a debugging
> > patch, that is, from big number to small number.
> > 
> > Signed-off-by: Huang, Ying <ying.huang@...el.com>
> > ---
> >  fs/f2fs/recovery.c |    7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > --- a/fs/f2fs/recovery.c
> > +++ b/fs/f2fs/recovery.c
> > @@ -172,8 +172,8 @@ static int find_fsync_dnodes(struct f2fs
> >  			if (IS_INODE(page) && is_dent_dnode(page))
> >  				set_inode_flag(F2FS_I(entry->inode),
> >  							FI_INC_LINK);
> > -		} else {
> > -			if (IS_INODE(page) && is_dent_dnode(page)) {
> 
> If this is not inode block, we should add this inode to recover its data blocks.

Is it possible that there is only non-inode dnode but no inode when
find_fsync_dnodes checking dnodes?  Per my understanding, any changes to
file will cause inode page dirty (for example, mtime changed), so that
we will write inode block.  Is it right?  If so, the solution in this
patch should work too.

Best Regards,
Huang, Ying

> Rather than this tweak, if iget is failed, we'd better go to next instead of
> break.
> Can you test that?
> 
> > +		} else if (IS_INODE(page)) {
> > +			if (is_dent_dnode(page)) {
> >  				err = recover_inode_page(sbi, page);
> >  				if (err)
> >  					break;
> > @@ -193,7 +193,8 @@ static int find_fsync_dnodes(struct f2fs
> >  				break;
> >  			}
> >  			list_add_tail(&entry->list, head);
> > -		}
> > +		} else
> > +			goto next;
> >  		entry->blkaddr = blkaddr;
> >  
> >  		err = recover_inode(entry->inode, page);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/