tcmu_rbd_open() detect/fail if block_size is zero
diff mbox

Message ID 20170504201728.25807-2-dab21774@gmail.com
State New, archived
Headers show

Commit Message

David Butterfield May 4, 2017, 8:17 p.m. UTC
Modify the check of the return from tcmu_get_attribute("hw_block_size")
to fail if the returned value is zero, so as to avoid dividing by it
a few lines later.

Signed-off-by: David Butterfield <dab21774@gmail.com>
---
 rbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mike Christie May 8, 2017, 5:15 p.m. UTC | #1
On 05/04/2017 03:17 PM, David Butterfield wrote:
> Modify the check of the return from tcmu_get_attribute("hw_block_size")
> to fail if the returned value is zero, so as to avoid dividing by it
> a few lines later.
> 
> Signed-off-by: David Butterfield <dab21774@gmail.com>
> ---
>  rbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rbd.c b/rbd.c
> index 852338d..c8019b5 100644
> --- a/rbd.c
> +++ b/rbd.c
> @@ -411,7 +411,7 @@ static int tcmu_rbd_open(struct tcmu_device *dev)
>  	config += 1; /* get past '/' */
>  
>  	block_size = tcmu_get_attribute(dev, "hw_block_size");
> -	if (block_size < 0) {
> +	if (block_size <= 0) {
>  		tcmu_dev_err(dev, "Could not get hw_block_size\n");
>  		ret = -EINVAL;
>  		goto free_state;
> 

Are you able to set the hw_block_size to 0? I am asking because it looks
like there are a couple checks in the kernel so this should not happen.
Is this with your our of tree use?

Patch seems harmless. Merged. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Butterfield May 10, 2017, 2:15 a.m. UTC | #2
I was having a problem because I was getting the value to return for
"hw_block_size" from a place that wasn't set yet.

I've fixed that, but still, it didn't seem good to go on to a obvious
divide by zero when it is so easy to avoid gracefully should it ever
happen.


On Mon, May 8, 2017 at 11:15 AM, Mike Christie <mchristi@redhat.com> wrote:
> On 05/04/2017 03:17 PM, David Butterfield wrote:
>> Modify the check of the return from tcmu_get_attribute("hw_block_size")
>> to fail if the returned value is zero, so as to avoid dividing by it
>> a few lines later.
>>
>> Signed-off-by: David Butterfield <dab21774@gmail.com>
>> ---
>>  rbd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/rbd.c b/rbd.c
>> index 852338d..c8019b5 100644
>> --- a/rbd.c
>> +++ b/rbd.c
>> @@ -411,7 +411,7 @@ static int tcmu_rbd_open(struct tcmu_device *dev)
>>       config += 1; /* get past '/' */
>>
>>       block_size = tcmu_get_attribute(dev, "hw_block_size");
>> -     if (block_size < 0) {
>> +     if (block_size <= 0) {
>>               tcmu_dev_err(dev, "Could not get hw_block_size\n");
>>               ret = -EINVAL;
>>               goto free_state;
>>
>
> Are you able to set the hw_block_size to 0? I am asking because it looks
> like there are a couple checks in the kernel so this should not happen.
> Is this with your our of tree use?
>
> Patch seems harmless. Merged. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe target-devel" 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/rbd.c b/rbd.c
index 852338d..c8019b5 100644
--- a/rbd.c
+++ b/rbd.c
@@ -411,7 +411,7 @@  static int tcmu_rbd_open(struct tcmu_device *dev)
 	config += 1; /* get past '/' */
 
 	block_size = tcmu_get_attribute(dev, "hw_block_size");
-	if (block_size < 0) {
+	if (block_size <= 0) {
 		tcmu_dev_err(dev, "Could not get hw_block_size\n");
 		ret = -EINVAL;
 		goto free_state;