linux-kernel - Re: bcache: unlock new_nodes[i]->write_lock in btree_gc

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]

Date:   Mon, 20 Apr 2020 15:10:30 +0800
From:   Coly Li <colyli@...e.de>
To:     Zhiqiang Liu <liuzhiqiang26@...wei.com>
Cc:     kmo@...erainc.com, linux-bcache@...r.kernel.org,
        linux-kernel@...r.kernel.org, "wubo (T)" <wubo40@...wei.com>,
        Mingfangsen <mingfangsen@...wei.com>,
        Yanxiaodan <yanxiaodan@...wei.com>,
        linfeilong <linfeilong@...wei.com>
Subject: Re: bcache: unlock new_nodes[i]->write_lock in btree_gc_coalesce

On 2020/4/20 14:55, Zhiqiang Liu wrote:
> friendly ping...
> 

The patch is good. Just it would be better if the commit log can be more
accurate and understandable for other readers who are not very familiar
with bcache code.

Thanks.


Coly Li


> On 2020/4/15 19:42, Zhiqiang Liu wrote:
>> From: Zhiqiang Liu <liuzhiqiang26@...wei.com>
>>
>> coccicheck reports:
>>   drivers/md//bcache/btree.c:1538:1-7: preceding lock on line 1417
>>
>> After obtaining new_nodes[i]->write_lock, we may go to out_nocoalesce
>> tag without releasing lock. Here, we add a new tag
>> 'out_unlock_nocoalesce' before out_nocoalesce tag to release
>> new_nodes[i]->write_lock.
>>
>> Fixes: 2a285686c1 ("bcache: btree locking rework")
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@...wei.com>
>> ---
>>  drivers/md/bcache/btree.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
>> index fa872df4e770..cad8b0b97e33 100644
>> --- a/drivers/md/bcache/btree.c
>> +++ b/drivers/md/bcache/btree.c
>> @@ -1447,7 +1447,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>  			if (__set_blocks(n1, n1->keys + n2->keys,
>>  					 block_bytes(b->c)) >
>>  			    btree_blocks(new_nodes[i]))
>> -				goto out_nocoalesce;
>> +				goto out_unlock_nocoalesce;
>>
>>  			keys = n2->keys;
>>  			/* Take the key of the node we're getting rid of */
>> @@ -1476,7 +1476,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>
>>  		if (__bch_keylist_realloc(&keylist,
>>  					  bkey_u64s(&new_nodes[i]->key)))
>> -			goto out_nocoalesce;
>> +			goto out_unlock_nocoalesce;
>>
>>  		bch_btree_node_write(new_nodes[i], &cl);
>>  		bch_keylist_add(&keylist, &new_nodes[i]->key);
>> @@ -1522,6 +1522,10 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>  	/* Invalidated our iterator */
>>  	return -EINTR;
>>
>> +out_unlock_nocoalesce:
>> +	for (i = 0; i < nodes; i++)
>> +		mutex_unlock(&new_nodes[i]->write_lock);
>> +
>>  out_nocoalesce:
>>  	closure_sync(&cl);
>>
>