linux-ext4 - Re: [PATCH] Ext2: do not mark_inode_dirty to avoid BUG

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]

Message-ID: <alpine.LFD.2.00.1303120904030.7128@dhcp-1-104.brq.redhat.com>
Date:	Tue, 12 Mar 2013 09:14:21 +0100 (CET)
From:	Lukáš Czerner <lczerner@...hat.com>
To:	fanchaoting <fanchaoting@...fujitsu.com>
cc:	jack@...e.cz, tyhicks@...onical.com, linux-ext4@...r.kernel.org,
	wangshilong1991@...il.com
Subject: Re: [PATCH] Ext2: do not mark_inode_dirty to avoid BUG_ON

On Tue, 12 Mar 2013, fanchaoting wrote:

> Date: Tue, 12 Mar 2013 13:06:37 +0800
> From: fanchaoting <fanchaoting@...fujitsu.com>
> To: jack@...e.cz
> Cc: tyhicks@...onical.com, linux-ext4@...r.kernel.org,
>     wangshilong1991@...il.com
> Subject: [PATCH] Ext2: do not mark_inode_dirty to avoid BUG_ON
> 
> From: Wang Shilong <wangsl-fnst@...fujitsu.com>
> 
> commit 8e3dffc651cb668e1ff4d8b89cc1c3dde7540d3b leads into
> a regression that casue BUG_ON when unlinking inode.

Hi,

it seems to be that we do need to mark the inode dirty, because
we're changing inode->i_blocks from within
dquot_free_block_nodirty().

However looking at the code we usually call mark_inode_dirty(inode)
after we call ext2_free_blocks() except when we're about to remove
the inode so it seems that having that call within ext2_free_blocks()
is not necessary.

However I am not sure about the error path in ext2_alloc_branch()
which does not dirty the inode after calling ext2_free_blocks().
However presumably since we're just undoing the changes we might
have done and not actually allocating, or freeing any space for
real, dirtying the inode might not be necessary. Can you confirm
that ?

Thanks!
-Lukas

> 
> Reported-by: tyhicks@...onical.com
> Signed-off-by: Wang Shilong <wangsl-fnst@...fujitsu.com>
> ---
>  fs/ext2/balloc.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
> index 9f9992b..06d82fc 100644
> --- a/fs/ext2/balloc.c
> +++ b/fs/ext2/balloc.c
> @@ -562,7 +562,6 @@ error_return:
>  	if (freed) {
>  		percpu_counter_add(&sbi->s_freeblocks_counter, freed);
>  		dquot_free_block_nodirty(inode, freed);
> -		mark_inode_dirty(inode);
>  	}
>  }
>  
> -- 1.7.7.6 
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html