From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Tsyvarev Subject: Re: f2fs: Possible use-after-free when umount filesystem Date: Thu, 24 Jul 2014 14:14:36 +0400 Message-ID: <53D0DC8C.7050406@ispras.ru> References: <52F320FC.50803@ispras.ru> <534BC29B.3020408@ispras.ru> <53CCF1EC.30008@ispras.ru> <53CDC9AF.2050605@cn.fujitsu.com> <53CE3722.60307@ispras.ru> <000001cfa61b$c693b350$53bb19f0$@samsung.com> <53CF2E61.3000601@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from sog-mx-3.v43.ch3.sourceforge.com ([172.29.43.193] helo=mx.sourceforge.net) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1XAG2G-0000Y8-IO for linux-f2fs-devel@lists.sourceforge.net; Thu, 24 Jul 2014 10:14:04 +0000 Received: from smtp.ispras.ru ([83.149.199.79]) by sog-mx-3.v43.ch3.sourceforge.com with esmtp (Exim 4.76) id 1XAG2D-00082G-GZ for linux-f2fs-devel@lists.sourceforge.net; Thu, 24 Jul 2014 10:14:04 +0000 In-Reply-To: <53CF2E61.3000601@cn.fujitsu.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: Gu Zheng , Chao Yu Cc: 'Jaegeuk Kim' , 'linux-kernel' , 'Alexey Khoroshilov' , linux-f2fs-devel@lists.sourceforge.net SGksCgpXaXRoIHBhdGNoIHNraXBwaW5nIGludmFsaWRhdGluZyBwYWdlcyBmb3Igbm9kZV9pbm9k ZSBhbmQgbWV0YV9pbm9kZSAKdXNlLWFmdGVyLWZyZWUgZXJyb3IgZGlzYXBwZWFycyB0b28uCgoy My4wNy4yMDE0IDc6MzksIEd1IFpoZW5nINC/0LjRiNC10YI6Cj4gSGksCj4gT24gMDcvMjMvMjAx NCAxMDoxMiBBTSwgQ2hhbyBZdSB3cm90ZToKPgo+PiBIaSBBbmRyZXkgR3UsCj4+Cj4+PiAtLS0t LU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQo+Pj4gRnJvbTogQW5kcmV5IFRzeXZhcmV2IFttYWlsdG86 dHN5dmFyZXZAaXNwcmFzLnJ1XQo+Pj4gU2VudDogVHVlc2RheSwgSnVseSAyMiwgMjAxNCA2OjA0 IFBNCj4+PiBUbzogR3UgWmhlbmcKPj4+IENjOiBKYWVnZXVrIEtpbTsgbGludXgta2VybmVsOyBB bGV4ZXkgS2hvcm9zaGlsb3Y7IGxpbnV4LWYyZnMtZGV2ZWxAbGlzdHMuc291cmNlZm9yZ2UubmV0 Cj4+PiBTdWJqZWN0OiBSZTogW2YyZnMtZGV2XSBmMmZzOiBQb3NzaWJsZSB1c2UtYWZ0ZXItZnJl ZSB3aGVuIHVtb3VudCBmaWxlc3lzdGVtCj4+Pgo+Pj4gSGkgR3UsCj4+Pgo+Pj4+PiBJbnZlc3Rp Z2F0aW9uIHNob3dzLCB0aGF0IGYyZnNfZXZpY3RfaW5vZGUsIHdoZW4gY2FsbGVkIGZvciAnbWV0 YV9pbm9kZScsIHVzZXMKPj4+IGludmFsaWRhdGVfbWFwcGluZ19wYWdlcygpIGZvciAnbm9kZV9p bm9kZScuCj4+Pj4+IEJ1dCAnbm9kZV9pbm9kZScgaXMgZGVsZXRlZCBiZWZvcmUgJ21ldGFfaW5v ZGUnIGluIGYyZnNfcHV0X3N1cGVyIHZpYSBpcHV0KCkuCj4+Pj4+Cj4+Pj4+IEl0IHNlZW1zIHRo YXQgaW4gY29tbW9uIHVzYWdlIHNjZW5hcmlvIHRoaXMgdXNlLWFmdGVyLWZyZWUgaXMgYmVuaWdu LCBiZWNhdXNlICdub2RlX2lub2RlJwo+Pj4gcmVtYWlucyBwYXJ0aWFsbHkgdmFsaWQgZGF0YSBl dmVuIGFmdGVyIGttZW1fY2FjaGVfZnJlZSgpLgo+Pj4+PiBCdXQgdGhpbmdzIG1heSBjaGFuZ2Ug aWYsIHdoaWxlICdtZXRhX2lub2RlJyBpcyBldmljdGVkIGluIG9uZSBmMmZzIGZpbGVzeXN0ZW0s IGFub3RoZXIgKG1vdW50ZWQpCj4+PiBmMmZzIGZpbGVzeXN0ZW0gcmVxdWVzdHMgaW5vZGUgZnJv bSBjYWNoZSwgYW5kIGZvcm1lbHkKPj4+Pj4gJ25vZGVfaW5vZGUnIG9mIHRoZSBmaXJzdCBmaWxl c3lzdGVtIGlzIHJldHVybmVkLgo+Pj4+IFRoZSBhbmFseXNpcyBzZWVtcyByZWFzb25hYmxlLiBI YXZlIHlvdSB0cmllZCB0byBzd2FwIHRoZSByZWNsYWltIG9yZGVyIG9mIG5vZGVfaW5kZQo+Pj4+ IGFuZCBtZXRhX2lub2RlPwo+Pj4+Cj4+Pj4gZGlmZiAtLWdpdCBhL2ZzL2YyZnMvc3VwZXIuYyBi L2ZzL2YyZnMvc3VwZXIuYwo+Pj4+IGluZGV4IDg3MGZlMTkuLmUxMTQ0MTggMTAwNjQ0Cj4+Pj4g LS0tIGEvZnMvZjJmcy9zdXBlci5jCj4+Pj4gKysrIGIvZnMvZjJmcy9zdXBlci5jCj4+Pj4gQEAg LTQzMCw4ICs0MzAsOCBAQCBzdGF0aWMgdm9pZCBmMmZzX3B1dF9zdXBlcihzdHJ1Y3Qgc3VwZXJf YmxvY2sgKnNiKQo+Pj4+ICAgICAgICAgICBpZiAoc2JpLT5zX2RpcnR5ICYmIGdldF9wYWdlcyhz YmksIEYyRlNfRElSVFlfTk9ERVMpKQo+Pj4+ICAgICAgICAgICAgICAgICAgIHdyaXRlX2NoZWNr cG9pbnQoc2JpLCB0cnVlKTsKPj4+Pgo+Pj4+IC0gICAgICAgaXB1dChzYmktPm5vZGVfaW5vZGUp Owo+Pj4+ICAgICAgICAgICBpcHV0KHNiaS0+bWV0YV9pbm9kZSk7Cj4+Pj4gKyAgICAgICBpcHV0 KHNiaS0+bm9kZV9pbm9kZSk7Cj4+Pj4KPj4+PiAgICAgICAgICAgLyogZGVzdHJveSBmMmZzIGlu dGVybmFsIG1vZHVsZXMgKi8KPj4+PiAgICAgICAgICAgZGVzdHJveV9ub2RlX21hbmFnZXIoc2Jp KTsKPj4+Pgo+Pj4+IFRoYW5rcywKPj4+PiBHdQo+Pj4gV2l0aCByZWNsYWltIG9yZGVyIG9mIG5v ZGVfaW5vZGUgYW5kIG1ldGFfaW5vZGUgc3dhcHBlZCwgdXNlLWFmdGVyLWZyZWUKPj4+IGVycm9y IGRpc2FwcGVhcnMuCj4+Pgo+Pj4gQnV0IHNob3VsZG4ndCBpbml0aWFsaXphdGlvbiBvcmRlciBv ZiB0aGVzZSBpbm9kZXMgYmUgc3dhcHBlZCB0b28/Cj4+PiBBcyBtZXRhX2lub2RlIHVzZXMgbm9k ZV9pbm9kZSwgaXQgc2VlbXMgbG9naWNhbCB0aGF0IGl0IHNob3VsZCBiZQo+Pj4gaW5pdGlhbGl6 ZWQgYWZ0ZXIgaXQuCj4gVGhlIGluaXRpYWxpemF0aW9uIG9yZGVyIGRvc2Ugbm90IGFmZmVjdCBh bnl0aGluZywgc28gc3dhcHBpbmcgdGhlIG9yZGVyIGRvc2Ugbm90Cj4gbWFrZSBtb3JlIHNlbnNl IGhlcmUuCj4KPj4gSU1PLCBpdCdzIG5vdCBlYXN5IHRvIGV4Y2hhbmdlIG9yZGVyIG9mIGluaXRp YWxpemF0aW9uIGJldHdlZW4gbWV0YV9pbm9kZSBhbmQKPj4gbm9kZV9pbm9kZSwgYmVjYXVzZSB3 ZSBzaG91bGQgdXNlIG1ldGFfaW5vZGUgaW4gZ2V0X3ZhbGlkX2NoZWNrcG9pbnQgZm9yIHZhbGlk Cj4+IGNwIGZpcnN0IGZvciB1c3VhbCB2ZXJpZmljYXRpb24sIHRoZW4gaW5pdCBub2RlX2lub2Rl Lgo+IFllYWgsIGJ1dCBJIHRoaW5rIGp1c3QgbW92aW5nIG5vZGVfaW5vZGUncyBpbml0aWFsaXph dGlvbiB0byB0aGUgZnJvbnQgb2YgbWV0YV9pbm9kZQo+IGRvc2Ugbm90IGJyZWFrIGFueXRoaW5n Lgo+Cj4+IEFzIEkgY2hlY2tlZCwgbmlkcyBmb3IgYm90aCBtZXRhX2lub2RlIGFuZCBub2RlX2lu b2RlIGFyZSByZXNlcnZhdGlvbiwgc28gaXQncyBub3QKPj4gbmVjZXNzYXJ5IGZvciB1cyB0byBp bnZhbGlkYXRlIHBhZ2VzIHdoaWNoIHdpbGwgbmV2ZXIgYWxsb2NlZC4KPj4KPj4gSG93IGFib3V0 IHNraXBwaW5nIGl0IGFzIGZvbGxvd2luZz8KPiBJdCBzZWVtcyB0aGUgcmlnaHQgd2F5IHRvIGZp eCB0aGlzIGlzc3VlLgo+Cj4gVG8gQW5kcmV5Ogo+IENvdWxkIHlvdSBwbGVhc2UgdHJ5IHRoaXMg b25lPwo+Cj4gVGhhbmtzLAo+IEd1Cj4KPj4gZGlmZiAtLWdpdCBhL2ZzL2YyZnMvaW5vZGUuYyBi L2ZzL2YyZnMvaW5vZGUuYwo+PiBpbmRleCAyY2Y2OTYyLi5jYWZiYTNjIDEwMDY0NAo+PiAtLS0g YS9mcy9mMmZzL2lub2RlLmMKPj4gKysrIGIvZnMvZjJmcy9pbm9kZS5jCj4+IEBAIC0yNzMsNyAr MjczLDcgQEAgdm9pZCBmMmZzX2V2aWN0X2lub2RlKHN0cnVjdCBpbm9kZSAqaW5vZGUpCj4+ICAg Cj4+ICAgCWlmIChpbm9kZS0+aV9pbm8gPT0gRjJGU19OT0RFX0lOTyhzYmkpIHx8Cj4+ICAgCQkJ aW5vZGUtPmlfaW5vID09IEYyRlNfTUVUQV9JTk8oc2JpKSkKPj4gLQkJZ290byBub19kZWxldGU7 Cj4+ICsJCWdvdG8gb3V0X2NsZWFyOwo+PiAgIAo+PiAgIAlmMmZzX2J1Z19vbihnZXRfZGlydHlf ZGVudHMoaW5vZGUpKTsKPj4gICAJcmVtb3ZlX2RpcnR5X2Rpcl9pbm9kZShpbm9kZSk7Cj4+IEBA IC0yOTUsNiArMjk1LDcgQEAgdm9pZCBmMmZzX2V2aWN0X2lub2RlKHN0cnVjdCBpbm9kZSAqaW5v ZGUpCj4+ICAgCj4+ICAgCXNiX2VuZF9pbnR3cml0ZShpbm9kZS0+aV9zYik7Cj4+ICAgbm9fZGVs ZXRlOgo+PiAtCWNsZWFyX2lub2RlKGlub2RlKTsKPj4gICAJaW52YWxpZGF0ZV9tYXBwaW5nX3Bh Z2VzKE5PREVfTUFQUElORyhzYmkpLCBpbm9kZS0+aV9pbm8sIGlub2RlLT5pX2lubyk7Cj4+ICtv dXRfY2xlYXI6Cj4+ICsJY2xlYXJfaW5vZGUoaW5vZGUpOwo+PiAgIH0KPj4KPj4+IC0tCj4+PiBC ZXN0IHJlZ2FyZHMsCj4+Pgo+Pj4gQW5kcmV5IFRzeXZhcmV2Cj4+PiBMaW51eCBWZXJpZmljYXRp b24gQ2VudGVyLCBJU1BSQVMKPj4+IHdlYjpodHRwOi8vbGludXh0ZXN0aW5nLm9yZwo+Pj4KPj4+ Cj4+PiAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPj4+IFdhbnQgZmFzdCBhbmQgZWFzeSBhY2Nlc3Mg dG8gYWxsIHRoZSBjb2RlIGluIHlvdXIgZW50ZXJwcmlzZT8gSW5kZXggYW5kCj4+PiBzZWFyY2gg dXAgdG8gMjAwLDAwMCBsaW5lcyBvZiBjb2RlIHdpdGggYSBmcmVlIGNvcHkgb2YgQmxhY2sgRHVj awo+Pj4gQ29kZSBTaWdodCAtIHRoZSBzYW1lIHNvZnR3YXJlIHRoYXQgcG93ZXJzIHRoZSB3b3Js ZCdzIGxhcmdlc3QgY29kZQo+Pj4gc2VhcmNoIG9uIE9obG9oLCB0aGUgQmxhY2sgRHVjayBPcGVu IEh1YiEgVHJ5IGl0IG5vdy4KPj4+IGh0dHA6Ly9wLnNmLm5ldC9zZnUvYmRzCj4+PiBfX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwo+Pj4gTGludXgtZjJmcy1k ZXZlbCBtYWlsaW5nIGxpc3QKPj4+IExpbnV4LWYyZnMtZGV2ZWxAbGlzdHMuc291cmNlZm9yZ2Uu bmV0Cj4+PiBodHRwczovL2xpc3RzLnNvdXJjZWZvcmdlLm5ldC9saXN0cy9saXN0aW5mby9saW51 eC1mMmZzLWRldmVsCj4+IC4KPj4KPgo+CgotLSAKQmVzdCByZWdhcmRzLAoKQW5kcmV5IFRzeXZh cmV2CkxpbnV4IFZlcmlmaWNhdGlvbiBDZW50ZXIsIElTUFJBUwp3ZWI6aHR0cDovL2xpbnV4dGVz dGluZy5vcmcKCgotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KV2FudCBmYXN0IGFuZCBlYXN5IGFjY2Vz cyB0byBhbGwgdGhlIGNvZGUgaW4geW91ciBlbnRlcnByaXNlPyBJbmRleCBhbmQKc2VhcmNoIHVw IHRvIDIwMCwwMDAgbGluZXMgb2YgY29kZSB3aXRoIGEgZnJlZSBjb3B5IG9mIEJsYWNrIER1Y2sK Q29kZSBTaWdodCAtIHRoZSBzYW1lIHNvZnR3YXJlIHRoYXQgcG93ZXJzIHRoZSB3b3JsZCdzIGxh cmdlc3QgY29kZQpzZWFyY2ggb24gT2hsb2gsIHRoZSBCbGFjayBEdWNrIE9wZW4gSHViISBUcnkg aXQgbm93LgpodHRwOi8vcC5zZi5uZXQvc2Z1L2JkcwpfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpMaW51eC1mMmZzLWRldmVsIG1haWxpbmcgbGlzdApMaW51 eC1mMmZzLWRldmVsQGxpc3RzLnNvdXJjZWZvcmdlLm5ldApodHRwczovL2xpc3RzLnNvdXJjZWZv cmdlLm5ldC9saXN0cy9saXN0aW5mby9saW51eC1mMmZzLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933543AbaGXKOK (ORCPT ); Thu, 24 Jul 2014 06:14:10 -0400 Received: from smtp.ispras.ru ([83.149.199.79]:50861 "EHLO smtp.ispras.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932943AbaGXKOH (ORCPT ); Thu, 24 Jul 2014 06:14:07 -0400 Message-ID: <53D0DC8C.7050406@ispras.ru> Date: Thu, 24 Jul 2014 14:14:36 +0400 From: Andrey Tsyvarev User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Gu Zheng , Chao Yu CC: "'Jaegeuk Kim'" , "'linux-kernel'" , "'Alexey Khoroshilov'" , linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] f2fs: Possible use-after-free when umount filesystem References: <52F320FC.50803@ispras.ru> <534BC29B.3020408@ispras.ru> <53CCF1EC.30008@ispras.ru> <53CDC9AF.2050605@cn.fujitsu.com> <53CE3722.60307@ispras.ru> <000001cfa61b$c693b350$53bb19f0$@samsung.com> <53CF2E61.3000601@cn.fujitsu.com> In-Reply-To: <53CF2E61.3000601@cn.fujitsu.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, With patch skipping invalidating pages for node_inode and meta_inode use-after-free error disappears too. 23.07.2014 7:39, Gu Zheng пишет: > Hi, > On 07/23/2014 10:12 AM, Chao Yu wrote: > >> Hi Andrey Gu, >> >>> -----Original Message----- >>> From: Andrey Tsyvarev [mailto:tsyvarev@ispras.ru] >>> Sent: Tuesday, July 22, 2014 6:04 PM >>> To: Gu Zheng >>> Cc: Jaegeuk Kim; linux-kernel; Alexey Khoroshilov; linux-f2fs-devel@lists.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. > The initialization order dose not affect anything, so swapping the order dose not > make more sense here. > >> 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. > Yeah, but I think just moving node_inode's initialization to the front of meta_inode > dose not break anything. > >> 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? > It seems the right way to fix this issue. > > To Andrey: > Could you please try this one? > > Thanks, > Gu > >> 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@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel >> . >> > > -- Best regards, Andrey Tsyvarev Linux Verification Center, ISPRAS web:http://linuxtesting.org