[PATCH] iio: inkern: fix a static checker error

[PATCH] iio: inkern: fix a static checker error

[Peter Rosin]

Avoid this smatch error:
drivers/iio/inkern.c:751 iio_read_avail_channel_raw() error: double unlock 'mutex:&chan->indio_dev->info_exist_lock'

Fixes: 00c5f80c2fad ("iio: inkern: add helpers to query available values from channels")
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/iio/inkern.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 7a13535dc3e9..a3941bade6a7 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -750,11 +750,9 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
 err_unlock:
 	mutex_unlock(&chan->indio_dev->info_exist_lock);
 
-	if (ret >= 0 && type != IIO_VAL_INT) {
+	if (ret >= 0 && type != IIO_VAL_INT)
 		/* raw values are assumed to be IIO_VAL_INT */
 		ret = -EINVAL;
-		goto err_unlock;
-	}
 
 	return ret;
 }
-- 
2.1.4

Re: [PATCH] iio: inkern: fix a static checker error

[Lars-Peter Clausen]

On 04/20/2017 11:01 PM, Peter Rosin wrote:
> Avoid this smatch error:
> drivers/iio/inkern.c:751 iio_read_avail_channel_raw() error: double unlock 'mutex:&chan->indio_dev->info_exist_lock'

Looks good, but it's not just the smatch error, this is a real issue. This
even seems to be a endless loop, always jumping back to err_unlock.

> 
> Fixes: 00c5f80c2fad ("iio: inkern: add helpers to query available values from channels")
> Signed-off-by: Peter Rosin <peda@axentia.se>

> ---
>  drivers/iio/inkern.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 7a13535dc3e9..a3941bade6a7 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -750,11 +750,9 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
>  err_unlock:
>  	mutex_unlock(&chan->indio_dev->info_exist_lock);
>  
> -	if (ret >= 0 && type != IIO_VAL_INT) {
> +	if (ret >= 0 && type != IIO_VAL_INT)
>  		/* raw values are assumed to be IIO_VAL_INT */
>  		ret = -EINVAL;
> -		goto err_unlock;
> -	}
>  
>  	return ret;
>  }
> 

Re: [PATCH] iio: inkern: fix a static checker error

[Peter Rosin]

On 2017-04-20 23:12, Lars-Peter Clausen wrote:
> On 04/20/2017 11:01 PM, Peter Rosin wrote:
>> Avoid this smatch error:
>> drivers/iio/inkern.c:751 iio_read_avail_channel_raw() error: double unlock 'mutex:&chan->indio_dev->info_exist_lock'
> 
> Looks good, but it's not just the smatch error, this is a real issue. This
> even seems to be a endless loop, always jumping back to err_unlock.

Yes, it should probably go to stable too...

>>
>> Fixes: 00c5f80c2fad ("iio: inkern: add helpers to query available values from channels")
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> 
>> ---
>>  drivers/iio/inkern.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index 7a13535dc3e9..a3941bade6a7 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -750,11 +750,9 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
>>  err_unlock:
>>  	mutex_unlock(&chan->indio_dev->info_exist_lock);
>>  
>> -	if (ret >= 0 && type != IIO_VAL_INT) {
>> +	if (ret >= 0 && type != IIO_VAL_INT)
>>  		/* raw values are assumed to be IIO_VAL_INT */
>>  		ret = -EINVAL;
>> -		goto err_unlock;
>> -	}
>>  
>>  	return ret;
>>  }
>>
> 

Re: [PATCH] iio: inkern: fix a static checker error

[Peter Rosin]

On 2017-04-20 23:13, Peter Rosin wrote:
> On 2017-04-20 23:12, Lars-Peter Clausen wrote:
>> On 04/20/2017 11:01 PM, Peter Rosin wrote:
>>> Avoid this smatch error:
>>> drivers/iio/inkern.c:751 iio_read_avail_channel_raw() error: double unlock 'mutex:&chan->indio_dev->info_exist_lock'
>>
>> Looks good, but it's not just the smatch error, this is a real issue. This
>> even seems to be a endless loop, always jumping back to err_unlock.
> 
> Yes, it should probably go to stable too...

Nope, not an endless loop, but I of course only noticed after sending
a v2 [1] which falsely stated just that. Ignore that v2 patch and take
this one instead, for the reasons stated in my followup [2] to that
message.

Involving stable is probably not needed either...

Cheers,
peda

[1] https://lkml.org/lkml/2017/4/24/179
[2] https://lkml.org/lkml/2017/4/24/221

>>>
>>> Fixes: 00c5f80c2fad ("iio: inkern: add helpers to query available values from channels")
>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>
>>> ---
>>>  drivers/iio/inkern.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>>> index 7a13535dc3e9..a3941bade6a7 100644
>>> --- a/drivers/iio/inkern.c
>>> +++ b/drivers/iio/inkern.c
>>> @@ -750,11 +750,9 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
>>>  err_unlock:
>>>  	mutex_unlock(&chan->indio_dev->info_exist_lock);
>>>  
>>> -	if (ret >= 0 && type != IIO_VAL_INT) {
>>> +	if (ret >= 0 && type != IIO_VAL_INT)
>>>  		/* raw values are assumed to be IIO_VAL_INT */
>>>  		ret = -EINVAL;
>>> -		goto err_unlock;
>>> -	}
>>>  
>>>  	return ret;
>>>  }
>>>
>>
> 

Re: [PATCH] iio: inkern: fix a static checker error

[Lars-Peter Clausen]

On 04/24/2017 11:32 AM, Peter Rosin wrote:
> On 2017-04-20 23:13, Peter Rosin wrote:
>> On 2017-04-20 23:12, Lars-Peter Clausen wrote:
>>> On 04/20/2017 11:01 PM, Peter Rosin wrote:
>>>> Avoid this smatch error:
>>>> drivers/iio/inkern.c:751 iio_read_avail_channel_raw() error: double unlock 'mutex:&chan->indio_dev->info_exist_lock'
>>>
>>> Looks good, but it's not just the smatch error, this is a real issue. This
>>> even seems to be a endless loop, always jumping back to err_unlock.
>>
>> Yes, it should probably go to stable too...
> 
> Nope, not an endless loop, but I of course only noticed after sending
> a v2 [1] which falsely stated just that. Ignore that v2 patch and take
> this one instead, for the reasons stated in my followup [2] to that
> message.
> 
> Involving stable is probably not needed either...

Right, my fault for sending you the wrong way. Sorry for that.

Re: [PATCH] iio: inkern: fix a static checker error

[Peter Rosin]

On 2017-04-25 18:01, Lars-Peter Clausen wrote:
> On 04/24/2017 11:32 AM, Peter Rosin wrote:
>> On 2017-04-20 23:13, Peter Rosin wrote:
>>> On 2017-04-20 23:12, Lars-Peter Clausen wrote:
>>>> On 04/20/2017 11:01 PM, Peter Rosin wrote:
>>>>> Avoid this smatch error:
>>>>> drivers/iio/inkern.c:751 iio_read_avail_channel_raw() error: double unlock 'mutex:&chan->indio_dev->info_exist_lock'
>>>>
>>>> Looks good, but it's not just the smatch error, this is a real issue. This
>>>> even seems to be a endless loop, always jumping back to err_unlock.
>>>
>>> Yes, it should probably go to stable too...
>>
>> Nope, not an endless loop, but I of course only noticed after sending
>> a v2 [1] which falsely stated just that. Ignore that v2 patch and take
>> this one instead, for the reasons stated in my followup [2] to that
>> message.
>>
>> Involving stable is probably not needed either...
> 
> Right, my fault for sending you the wrong way. Sorry for that.

No no, don't worry, I was already down that path without your help.

I just thought everybody would see the obvious bug and the urgency
when it was pointed out to them with a patch. Like you did. And I
was in a hurry for it to have a slim chance of getting in before
4.12 (but then -rc8 happened instead) so didn't spend a lot of time
crafting a thorough commit message (nor analyzing the problem, but
why would I when I had a very clear picture of an endless loop on
error?). But in the end it wasn't really that obvious, was it? And
not that urgent either. Stupid damn bug...

Cheers,
peda

Re: [PATCH] iio: inkern: fix a static checker error

[Jonathan Cameron]

On 24/04/17 10:32, Peter Rosin wrote:
> On 2017-04-20 23:13, Peter Rosin wrote:
>> On 2017-04-20 23:12, Lars-Peter Clausen wrote:
>>> On 04/20/2017 11:01 PM, Peter Rosin wrote:
>>>> Avoid this smatch error:
>>>> drivers/iio/inkern.c:751 iio_read_avail_channel_raw() error: double unlock 'mutex:&chan->indio_dev->info_exist_lock'
>>>
>>> Looks good, but it's not just the smatch error, this is a real issue. This
>>> even seems to be a endless loop, always jumping back to err_unlock.
>>
>> Yes, it should probably go to stable too...
> 
> Nope, not an endless loop, but I of course only noticed after sending
> a v2 [1] which falsely stated just that. Ignore that v2 patch and take
> this one instead, for the reasons stated in my followup [2] to that
> message.
> 
> Involving stable is probably not needed either...
Applied to the togreg branch of iio.git.  Thanks,

Jonathan
> 
> Cheers,
> peda
> 
> [1] https://lkml.org/lkml/2017/4/24/179
> [2] https://lkml.org/lkml/2017/4/24/221
> 
>>>>
>>>> Fixes: 00c5f80c2fad ("iio: inkern: add helpers to query available values from channels")
>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>
>>>> ---
>>>>  drivers/iio/inkern.c | 4 +---
>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>>>> index 7a13535dc3e9..a3941bade6a7 100644
>>>> --- a/drivers/iio/inkern.c
>>>> +++ b/drivers/iio/inkern.c
>>>> @@ -750,11 +750,9 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
>>>>  err_unlock:
>>>>  	mutex_unlock(&chan->indio_dev->info_exist_lock);
>>>>  
>>>> -	if (ret >= 0 && type != IIO_VAL_INT) {
>>>> +	if (ret >= 0 && type != IIO_VAL_INT)
>>>>  		/* raw values are assumed to be IIO_VAL_INT */
>>>>  		ret = -EINVAL;
>>>> -		goto err_unlock;
>>>> -	}
>>>>  
>>>>  	return ret;
>>>>  }
>>>>
>>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>