linux-kernel - Re: [PATCH] ext4: Do not destroy ext4_groupinfo_caches if ext4_mb

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] [day] [month] [year] [list]

Date:	Tue, 13 May 2014 12:25:23 +0200 (CEST)
From:	Lukáš Czerner <lczerner@...hat.com>
To:	Andrey Tsyvarev <tsyvarev@...ras.ru>
cc:	"Theodore Ts'o" <tytso@....edu>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
	Alexey Khoroshilov <khoroshilov@...ras.ru>
Subject: Re: [PATCH] ext4: Do not destroy ext4_groupinfo_caches if ext4_mb_init()
 fails

On Tue, 13 May 2014, Andrey Tsyvarev wrote:

> Date: Tue, 13 May 2014 14:17:25 +0400
> From: Andrey Tsyvarev <tsyvarev@...ras.ru>
> To: Lukáš Czerner <lczerner@...hat.com>
> Cc: Theodore Ts'o <tytso@....edu>, Andreas Dilger <adilger.kernel@...ger.ca>,
>     linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
>     Alexey Khoroshilov <khoroshilov@...ras.ru>
> Subject: Re: [PATCH] ext4: Do not destroy ext4_groupinfo_caches if
>     ext4_mb_init() fails
> 
> 
> 12.05.2014 19:08, Lukáš Czerner пишет:
> > On Mon, 12 May 2014, Andrey Tsyvarev wrote:
> > 
> > > Date: Mon, 12 May 2014 12:23:59 +0400
> > > From: Andrey Tsyvarev<tsyvarev@...ras.ru>
> > > To: Theodore Ts'o<tytso@....edu>
> > > Cc: Andrey Tsyvarev<tsyvarev@...ras.ru>,
> > >      Andreas Dilger<adilger.kernel@...ger.ca>,linux-ext4@...r.kernel.org,
> > >      linux-kernel@...r.kernel.org, Alexey
> > > Khoroshilov<khoroshilov@...ras.ru>
> > > Subject: [PATCH] ext4: Do not destroy ext4_groupinfo_caches if
> > > ext4_mb_init()
> > >      fails
> > > 
> > > Caches from 'ext4_groupinfo_caches' may be in use by other mounts, which
> > > have already existed.
> > > So, it is incorrect to destroy them when newly requested mount fails.
> > > 
> > > Found by Linux File System Verification project (linuxtesting.org).
> > Makes sense, thanks! Can you please share the test case which
> > triggered this ? It might be worth including in xfstests.
> 
> Actually it was triggered by xfstests themselves but run with fault
> simulation.
> The method of fault simulation is under development/evaluation now, we expect
> to publish a paper describing it in the near future.
> 
> BUG_ON() in get_groupinfo_cache() was firstly triggered by test generic/003,
> but actually it could be any other test, which uses a scratch device: xftests
> itself requires test device(TEST_DEV) mounted, so a fault simulated while
> mount scratch device causes the problem described.

It sounds interesting. I hope that you'll send the information out
to the fsdevel list when your paper is finished, It looks like it
might be quite useful.

Thanks!
-Lukas

> 
> 
> > Reviewed-by: Lukas Czerner<lczerner@...hat.com>
> > 
> > > Signed-off-by: Andrey Tsyvarev<tsyvarev@...ras.ru>
> > > ---
> > >   fs/ext4/mballoc.c | 4 +---
> > >   1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> > > index 04a5c75..becea1d 100644
> > > --- a/fs/ext4/mballoc.c
> > > +++ b/fs/ext4/mballoc.c
> > > @@ -2607,7 +2607,7 @@ int ext4_mb_init(struct super_block *sb)
> > >   	sbi->s_locality_groups = alloc_percpu(struct ext4_locality_group);
> > >   	if (sbi->s_locality_groups == NULL) {
> > >   		ret = -ENOMEM;
> > > -		goto out_free_groupinfo_slab;
> > > +		goto out;
> > >   	}
> > >   	for_each_possible_cpu(i) {
> > >   		struct ext4_locality_group *lg;
> > > @@ -2632,8 +2632,6 @@ int ext4_mb_init(struct super_block *sb)
> > >   out_free_locality_groups:
> > >   	free_percpu(sbi->s_locality_groups);
> > >   	sbi->s_locality_groups = NULL;
> > > -out_free_groupinfo_slab:
> > > -	ext4_groupinfo_destroy_slabs();
> > >   out:
> > >   	kfree(sbi->s_mb_offsets);
> > >   	sbi->s_mb_offsets = NULL;
> > > 
> 
> --
> Andrey Tsyvarev
> Linux Verification Center, ISPRAS
> 
>