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

Message-ID: <c18d6aa20912151516g3ad5e0a6iccaaa3a77a7a5f63@mail.gmail.com>
Date:	Wed, 16 Dec 2009 02:16:36 +0300
From:	Dmitry Monakhov <rjevskiy@...il.com>
To:	Jan Kara <jack@...e.cz>
Cc:	linux-ext4@...r.kernel.org, cmm@...ibm.com
Subject: Re: [PATCH] ext4: fix sleep inside spinlock issue aka #14739 V2

2009/12/16 Jan Kara <jack@...e.cz>:
> On Thu 10-12-09 20:22:16, 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
>  Dmitry, I suppose I should also merge this patch together with your other
> fixes, right? For some reason, it was not part of the last submission of
> your patch series...
yes. it is not in the series because it is another bug, and it is not
depended from
other patches.
Please merge newst version of the patch http://patchwork.ozlabs.org/patch/40896/
This version is affected by stupid bug, see at the bottom of the patch.
In fact this patch does help Justin Maggard to overcome his issue.

BTW  i've also have another patch http://patchwork.ozlabs.org/patch/40805/
it was acked by you, but ASAIK it was't pushed to the ext4 queue yet.
>
>                                                                Honza
>
>> 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;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
meta_blocks may be changed after we dropped the lock, so we have to
use add here:
EXT4_I(inode)->i_reserved_meta_blocks += mb_needed;
I've overlooked this simple bug, and in fact i'm able to catch it only
with havy io load
./fsstress -p16 .....
>> -
>>       spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>> +
>>       return 0;       /* success */
>>  }
>>
>> --
>> 1.6.0.4
>>
> --
> Jan Kara <jack@...e.cz>
> SUSE Labs, CR
>
--
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