Re: [PATCH] HID: steelseries: Fix signedness bug in steelseries_headset_arctis_1_fetch_battery()
From: Bastien Nocera <hadess@hadess.net>
Date: 2023-09-13 14:43:33
Also in:
kernel-janitors
Hey Dan, On Thu, 2023-09-07 at 12:55 +0300, Dan Carpenter wrote:
The hid_hw_raw_request() function returns negative error codes or the number bytes transferred. If it returns negative error codes, then the problem is that when we check if "ret < sizeof(arctis_1_battery_request)", negative values are type promoted to high unsigned values and treated as success. Add an explicit check for negatives to address this issue.
I would be grateful if you could either add the compiler's error message or explain in greater details the integer type promotion (or demotion from signed to unsigned in this case) in the commit message.
Fixes: a0c76896c3fb ("HID: steelseries: Add support for Arctis 1
XBox")
Signed-off-by: Dan Carpenter <redacted>quoted hunk ↗ jump to hunk
--- 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..485d2287d58a 100644--- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c@@ -390,7 +390,7 @@ static intsteelseries_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 < 0 || ret < sizeof(arctis_1_battery_request)) {
I prefer:
- if (ret < sizeof(arctis_1_battery_request)) {
+ if (ret < (int) sizeof(arctis_1_battery_request)) {
although I'm not sure that's the kernel-style for this sort of checks.
hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); ret = -ENODATA; }