Hi Kim, > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk....@samsung.com] > Sent: Monday, December 23, 2013 9:26 AM > To: Chao Yu > Cc: linux-fsde...@vger.kernel.org; linux-ker...@vger.kernel.org; > linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH 1/3] f2fs: check filename length in > recover_dentry > > 2013-12-21 (토), 18:01 +0800, Chao Yu: > > In current flow, we will get Null return value of f2fs_find_entry in > > recover_dentry when name.len is bigger than F2FS_NAME_LEN, and then we > > still add this inode into its dir entry. > > To avoid this situation, we must check filename length before we use it. > > > > Another point is that we could remove the code of checking filename length > > In f2fs_find_entry, because f2fs_lookup will be called previously to ensure > > of > > validity of filename length. > > > > Signed-off-by: Chao Yu <chao2...@samsung.com> > > --- > > fs/f2fs/dir.c | 3 --- > > fs/f2fs/recovery.c | 5 +++++ > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c > > index 0cc26ba..3f3b661 100644 > > --- a/fs/f2fs/dir.c > > +++ b/fs/f2fs/dir.c > > @@ -190,9 +190,6 @@ struct f2fs_dir_entry *f2fs_find_entry(struct inode > > *dir, > > unsigned int max_depth; > > unsigned int level; > > > > - if (unlikely(namelen > F2FS_NAME_LEN)) > > - return NULL; > > - > > if (npages == 0) > > return NULL; > > > > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c > > index a3f4542..fdd175b 100644 > > --- a/fs/f2fs/recovery.c > > +++ b/fs/f2fs/recovery.c > > @@ -62,6 +62,11 @@ static int recover_dentry(struct page *ipage, struct > > inode *inode) > > > > name.len = le32_to_cpu(raw_inode->i_namelen); > > name.name = raw_inode->i_name; > > + > > + if (unlikely(name.len > F2FS_NAME_LEN)) { > > + err = -ENAMETOOLONG; > > + goto out; > > + } > > Have you seen this before?
Not yet. > This is a trivial bug case, so, if you have got this bug, we should fix > the bug first instead of adding any workaround patch. What I worry about is that not only f2fs bug lead to this trivial problem, but also other program with operation in raw disk could do this. > Let's add WARN_ON() at least. Alright. Thanks. > Thanks, > > > retry: > > de = f2fs_find_entry(dir, &name, &page); > > if (de && inode->i_ino == le32_to_cpu(de->ino)) > > -- > Jaegeuk Kim > Samsung ------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
