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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <c18d6aa20912190711u1c66973dp78d9bdc63b77437a@mail.gmail.com>
Date:	Sat, 19 Dec 2009 18:11:51 +0300
From:	Dmitry Monakhov <rjevskiy@...il.com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
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

2009/12/18 Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>:
> 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.
Yes. you right. I've already fixed this.
Please take a look at committed version:
http://git.kernel.org/?p=linux/kernel/git/jack/linux-fs-2.6.git;a=commitdiff;h=ef22d6deda461ef32c72944f662863c022253571
>
> -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