| Message ID | 20181019091909.a22ah7wybj6agqch@kili.mountain (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [1/3] scsi: myrs: Fix a logical vs bitwise bug | expand |
Dan, > We only want the value to be zero or one. > > It's not a big deal, but say we passed set value to INT_MIN, then > disable_enclosure_messages_show() would return that 12 bytes of "buf" > are initialized but actually only 3 are. I think there are tools like > KASAN which will trigger an info leak warning when that happens. s/kstrtoint/kstrtouint/?
On Fri, Oct 19, 2018 at 12:19:09PM +0300, Dan Carpenter wrote: > We only want the value to be zero or one. > > It's not a big deal, but say we passed set value to INT_MIN, then > disable_enclosure_messages_show() would return that 12 bytes of "buf" > are initialized but actually only 3 are. I think there are tools like > KASAN which will trigger an info leak warning when that happens. > > Fixes: 77266186397c ("scsi: myrs: Add Mylex RAID controller (SCSI interface)") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/scsi/myrs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c > index 07e5a3f54e31..55842ed54231 100644 > --- a/drivers/scsi/myrs.c > +++ b/drivers/scsi/myrs.c > @@ -1501,7 +1501,7 @@ static ssize_t disable_enclosure_messages_store(struct device *dev, > if (ret) > return ret; > > - if (value > 2) > + if (value < 0 || value > 2) > return -EINVAL; It's not actually clear to me why we allow 2. Shouldn't we just use kstrtobool()? regards, dan carpenter
Dan, >> + if (value < 0 || value > 2) >> return -EINVAL; > > It's not actually clear to me why we allow 2. Shouldn't we just use > kstrtobool()? Hannes?
diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c index 07e5a3f54e31..55842ed54231 100644 --- a/drivers/scsi/myrs.c +++ b/drivers/scsi/myrs.c @@ -1501,7 +1501,7 @@ static ssize_t disable_enclosure_messages_store(struct device *dev, if (ret) return ret; - if (value > 2) + if (value < 0 || value > 2) return -EINVAL; cs->disable_enc_msg = value;
We only want the value to be zero or one. It's not a big deal, but say we passed set value to INT_MIN, then disable_enclosure_messages_show() would return that 12 bytes of "buf" are initialized but actually only 3 are. I think there are tools like KASAN which will trigger an info leak warning when that happens. Fixes: 77266186397c ("scsi: myrs: Add Mylex RAID controller (SCSI interface)") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/scsi/myrs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)