| Message ID | 20170504201728.25807-2-dab21774@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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;
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(-)