linux-ext4 - Re: [PATCH] ext4: fix sleep inside spinlock issue aka #14739 V2

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091218100603.GC9437@skywalker.linux.vnet.ibm.com>
Date:	Fri, 18 Dec 2009 15:36:03 +0530
From:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	Dmitry Monakhov <dmonakhov@...nvz.org>
Cc:	linux-ext4@...r.kernel.org, Jan Kara <jack@...e.cz>, cmm@...ibm.com
Subject: Re: [PATCH] ext4: fix sleep inside spinlock issue aka #14739 V2

On Thu, Dec 10, 2009 at 08:22:16PM +0300, Dmitry Monakhov wrote:
> 
> drop i_block_reservation_lock before vfs_dq_reserve_block().
> this patch fix http://bugzilla.kernel.org/show_bug.cgi?id=14739
> 
> changes from previous version:
>  - simplify the patch according to Jan's comments
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@...nvz.org>
> ---
>  fs/ext4/inode.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 942e183..2327f7a 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1851,6 +1851,7 @@ repeat:
> 
>  	md_needed = mdblocks - EXT4_I(inode)->i_reserved_meta_blocks;
>  	total = md_needed + nrblocks;
> +	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> 
>  	/*
>  	 * Make quota reservation here to prevent quota overflow
> @@ -1858,12 +1859,10 @@ repeat:
>  	 * time.
>  	 */
>  	if (vfs_dq_reserve_block(inode, total)) {
> -		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  		return -EDQUOT;
>  	}
> 
>  	if (ext4_claim_free_blocks(sbi, total)) {
> -		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  		vfs_dq_release_reservation_block(inode, total);
>  		if (ext4_should_retry_alloc(inode->i_sb, &retries)) {
>  			yield();
> @@ -1871,10 +1870,11 @@ repeat:
>  		}
>  		return -ENOSPC;
>  	}
> +	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
>  	EXT4_I(inode)->i_reserved_data_blocks += nrblocks;
>  	EXT4_I(inode)->i_reserved_meta_blocks = mdblocks;
> -
>  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> +
>  	return 0;       /* success */


NACK
I guess we would end up setting i_reserved_meta_blocks wrongly because
mdblocks could be based on the old value of i_reserved_meta_blocks
because we are dropping i_block_reservation_lock lock.

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