blkcg: Unlock blkcg_pol_mutex once if cpd == NULL
diff mbox

Message ID db0c6205-9ca2-d80e-26fc-6cff0d84d73c@sandisk.com
State New
Headers show

Commit Message

Bart Van Assche Sept. 26, 2016, 10:36 p.m. UTC
Unlocking a mutex twice is wrong. Hence modify blkcg_policy_register()
such that blkcg_pol_mutex is unlocked once if cpd == NULL. This patch
avoids that smatch reports the following error:

block/blk-cgroup.c:1378: blkcg_policy_register() error: double unlock 'mutex:&blkcg_pol_mutex'

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: <stable@vger.kernel.org>
---
 block/blk-cgroup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Tejun Heo Sept. 29, 2016, 10:03 a.m. UTC | #1
Hello,

On Mon, Sep 26, 2016 at 03:36:25PM -0700, Bart Van Assche wrote:
> Unlocking a mutex twice is wrong. Hence modify blkcg_policy_register()
> such that blkcg_pol_mutex is unlocked once if cpd == NULL. This patch
> avoids that smatch reports the following error:
> 
> block/blk-cgroup.c:1378: blkcg_policy_register() error: double unlock 'mutex:&blkcg_pol_mutex'
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: <stable@vger.kernel.org>
> ---
>  block/blk-cgroup.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index dd38e5c..cdbca1c 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -1327,8 +1327,10 @@ int blkcg_policy_register(struct blkcg_policy *pol)
>  	for (i = 0; i < BLKCG_MAX_POLS; i++)
>  		if (!blkcg_policy[i])
>  			break;
> -	if (i >= BLKCG_MAX_POLS)
> +	if (i >= BLKCG_MAX_POLS) {
> +		mutex_unlock(&blkcg_pol_mutex);
>  		goto err_unlock;
> +	}

Wouldn't it be better to drop explicit mutx_unlock(&blkcg_pol_mutex)
on "if (!cpd)"?

Thanks.
Bart Van Assche Sept. 29, 2016, 3:32 p.m. UTC | #2
On 09/29/2016 03:03 AM, Tejun Heo wrote:
> Hello,
>
> On Mon, Sep 26, 2016 at 03:36:25PM -0700, Bart Van Assche wrote:
>> Unlocking a mutex twice is wrong. Hence modify blkcg_policy_register()
>> such that blkcg_pol_mutex is unlocked once if cpd == NULL. This patch
>> avoids that smatch reports the following error:
>>
>> block/blk-cgroup.c:1378: blkcg_policy_register() error: double unlock 'mutex:&blkcg_pol_mutex'
>>
>> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
>> Cc: Tejun Heo <tj@kernel.org>
>> Cc: <stable@vger.kernel.org>
>> ---
>>  block/blk-cgroup.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
>> index dd38e5c..cdbca1c 100644
>> --- a/block/blk-cgroup.c
>> +++ b/block/blk-cgroup.c
>> @@ -1327,8 +1327,10 @@ int blkcg_policy_register(struct blkcg_policy *pol)
>>  	for (i = 0; i < BLKCG_MAX_POLS; i++)
>>  		if (!blkcg_policy[i])
>>  			break;
>> -	if (i >= BLKCG_MAX_POLS)
>> +	if (i >= BLKCG_MAX_POLS) {
>> +		mutex_unlock(&blkcg_pol_mutex);
>>  		goto err_unlock;
>> +	}
>
> Wouldn't it be better to drop explicit mutx_unlock(&blkcg_pol_mutex)
> on "if (!cpd)"?

Agreed. I will rework this patch accordingly. Thanks for the feedback.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index dd38e5c..cdbca1c 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1327,8 +1327,10 @@  int blkcg_policy_register(struct blkcg_policy *pol)
 	for (i = 0; i < BLKCG_MAX_POLS; i++)
 		if (!blkcg_policy[i])
 			break;
-	if (i >= BLKCG_MAX_POLS)
+	if (i >= BLKCG_MAX_POLS) {
+		mutex_unlock(&blkcg_pol_mutex);
 		goto err_unlock;
+	}
 
 	/* register @pol */
 	pol->plid = i;
@@ -1374,8 +1376,8 @@  err_free_cpds:
 		}
 	}
 	blkcg_policy[pol->plid] = NULL;
+
 err_unlock:
-	mutex_unlock(&blkcg_pol_mutex);
 	mutex_unlock(&blkcg_pol_register_mutex);
 	return ret;
 }