| Message ID | 6e0a33e3-1564-419a-9946-b2d0afa0f98d@moroto.mountain (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 6c667ef6e2c88523469a6a94493b441cac2970d9 |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [v2] HID: steelseries: Fix signedness bug in steelseries_headset_arctis_1_fetch_battery() | expand |
On Fri, 2023-09-15 at 15:59 +0300, Dan Carpenter wrote: > The hid_hw_raw_request() function returns negative error codes or the > number bytes transferred. The problem is that when it returns > negative > error codes and we check if "ret < sizeof(arctis_1_battery_request)", > then the negative values are type promoted from int to high unsigned > long > values and treated as success. > > This was detected using Smatch: > > drivers/hid/hid-steelseries.c:393 > steelseries_headset_arctis_1_fetch_battery() > warn: error code type promoted to positive: 'ret' > > Fixes: a0c76896c3fb ("HID: steelseries: Add support for Arctis 1 > XBox") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Reviewed-by: Bastien Nocera <hadess@hadess.net> Thanks Dan! > --- > v2: Re-word the commit message. Add the Smatch warning. Use a cast > instead of an explicit check for negatives. > > drivers/hid/hid-steelseries.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid- > steelseries.c > index 43d2cf7153d7..b3edadf42d6d 100644 > --- a/drivers/hid/hid-steelseries.c > +++ b/drivers/hid/hid-steelseries.c > @@ -390,7 +390,7 @@ static int > steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev) > ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], > write_buf, > sizeof(arctis_1_battery_request), > HID_OUTPUT_REPORT, > HID_REQ_SET_REPORT); > - if (ret < sizeof(arctis_1_battery_request)) { > + if (ret < (int)sizeof(arctis_1_battery_request)) { > hid_err(hdev, "hid_hw_raw_request() failed with > %d\n", ret); > ret = -ENODATA; > }
On Fri, 15 Sep 2023, Dan Carpenter wrote: > The hid_hw_raw_request() function returns negative error codes or the > number bytes transferred. The problem is that when it returns negative > error codes and we check if "ret < sizeof(arctis_1_battery_request)", > then the negative values are type promoted from int to high unsigned long > values and treated as success. > > This was detected using Smatch: > > drivers/hid/hid-steelseries.c:393 steelseries_headset_arctis_1_fetch_battery() > warn: error code type promoted to positive: 'ret' > > Fixes: a0c76896c3fb ("HID: steelseries: Add support for Arctis 1 XBox") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > v2: Re-word the commit message. Add the Smatch warning. Use a cast > instead of an explicit check for negatives. > > drivers/hid/hid-steelseries.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c > index 43d2cf7153d7..b3edadf42d6d 100644 > --- a/drivers/hid/hid-steelseries.c > +++ b/drivers/hid/hid-steelseries.c > @@ -390,7 +390,7 @@ static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev) > ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], > write_buf, sizeof(arctis_1_battery_request), > HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); > - if (ret < sizeof(arctis_1_battery_request)) { > + if (ret < (int)sizeof(arctis_1_battery_request)) { > hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); > ret = -ENODATA; > } Applied, thanks.
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c index 43d2cf7153d7..b3edadf42d6d 100644 --- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c @@ -390,7 +390,7 @@ static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev) ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], write_buf, sizeof(arctis_1_battery_request), HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); - if (ret < sizeof(arctis_1_battery_request)) { + if (ret < (int)sizeof(arctis_1_battery_request)) { hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); ret = -ENODATA; }
The hid_hw_raw_request() function returns negative error codes or the number bytes transferred. The problem is that when it returns negative error codes and we check if "ret < sizeof(arctis_1_battery_request)", then the negative values are type promoted from int to high unsigned long values and treated as success. This was detected using Smatch: drivers/hid/hid-steelseries.c:393 steelseries_headset_arctis_1_fetch_battery() warn: error code type promoted to positive: 'ret' Fixes: a0c76896c3fb ("HID: steelseries: Add support for Arctis 1 XBox") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- v2: Re-word the commit message. Add the Smatch warning. Use a cast instead of an explicit check for negatives. drivers/hid/hid-steelseries.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)