linux-kernel - RE: [f2fs-dev] f2fs: Possible use-after-free when umount filesystem

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:	Wed, 23 Jul 2014 10:12:52 +0800
From:	Chao Yu <chao2.yu@...sung.com>
To:	'Andrey Tsyvarev' <tsyvarev@...ras.ru>,
	'Gu Zheng' <guz.fnst@...fujitsu.com>
Cc:	'Jaegeuk Kim' <jaegeuk@...nel.org>,
	'linux-kernel' <linux-kernel@...r.kernel.org>,
	'Alexey Khoroshilov' <khoroshilov@...ras.ru>,
	linux-f2fs-devel@...ts.sourceforge.net
Subject: RE: [f2fs-dev] f2fs: Possible use-after-free when umount filesystem

Hi Andrey Gu,

> -----Original Message-----
> From: Andrey Tsyvarev [mailto:tsyvarev@...ras.ru]
> Sent: Tuesday, July 22, 2014 6:04 PM
> To: Gu Zheng
> Cc: Jaegeuk Kim; linux-kernel; Alexey Khoroshilov; linux-f2fs-devel@...ts.sourceforge.net
> Subject: Re: [f2fs-dev] f2fs: Possible use-after-free when umount filesystem
> 
> Hi Gu,
> 
> >> Investigation shows, that f2fs_evict_inode, when called for 'meta_inode', uses
> invalidate_mapping_pages() for 'node_inode'.
> >> But 'node_inode' is deleted before 'meta_inode' in f2fs_put_super via iput().
> >>
> >> It seems that in common usage scenario this use-after-free is benign, because 'node_inode'
> remains partially valid data even after kmem_cache_free().
> >> But things may change if, while 'meta_inode' is evicted in one f2fs filesystem, another (mounted)
> f2fs filesystem requests inode from cache, and formely
> >> 'node_inode' of the first filesystem is returned.
> > The analysis seems reasonable. Have you tried to swap the reclaim order of node_inde
> > and meta_inode?
> >
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 870fe19..e114418 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -430,8 +430,8 @@ static void f2fs_put_super(struct super_block *sb)
> >          if (sbi->s_dirty && get_pages(sbi, F2FS_DIRTY_NODES))
> >                  write_checkpoint(sbi, true);
> >
> > -       iput(sbi->node_inode);
> >          iput(sbi->meta_inode);
> > +       iput(sbi->node_inode);
> >
> >          /* destroy f2fs internal modules */
> >          destroy_node_manager(sbi);
> >
> > Thanks,
> > Gu
> 
> With reclaim order of node_inode and meta_inode swapped, use-after-free
> error disappears.
> 
> But shouldn't initialization order of these inodes be swapped too?
> As meta_inode uses node_inode, it seems logical that it should be
> initialized after it.

IMO, it's not easy to exchange order of initialization between meta_inode and
node_inode, because we should use meta_inode in get_valid_checkpoint for valid
cp first for usual verification, then init node_inode.

As I checked, nids for both meta_inode and node_inode are reservation, so it's not
necessary for us to invalidate pages which will never alloced.

How about skipping it as following?

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 2cf6962..cafba3c 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -273,7 +273,7 @@ void f2fs_evict_inode(struct inode *inode)
 
 	if (inode->i_ino == F2FS_NODE_INO(sbi) ||
 			inode->i_ino == F2FS_META_INO(sbi))
-		goto no_delete;
+		goto out_clear;
 
 	f2fs_bug_on(get_dirty_dents(inode));
 	remove_dirty_dir_inode(inode);
@@ -295,6 +295,7 @@ void f2fs_evict_inode(struct inode *inode)
 
 	sb_end_intwrite(inode->i_sb);
 no_delete:
-	clear_inode(inode);
 	invalidate_mapping_pages(NODE_MAPPING(sbi), inode->i_ino, inode->i_ino);
+out_clear:
+	clear_inode(inode);
 }

> 
> --
> Best regards,
> 
> Andrey Tsyvarev
> Linux Verification Center, ISPRAS
> web:http://linuxtesting.org
> 
> 
> ------------------------------------------------------------------------------
> Want fast and easy access to all the code in your enterprise? Index and
> search up to 200,000 lines of code with a free copy of Black Duck
> Code Sight - the same software that powers the world's largest code
> search on Ohloh, the Black Duck Open Hub! Try it now.
> http://p.sf.net/sfu/bds
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

--
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/