[PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

[PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

[Dan Elkouby]

hidp_send_message was changed to return non-zero values on success,
which some other bits did not expect. This caused spurious errors to be
propagated through the stack, breaking some drivers, such as hid-sony
for the Dualshock 4 in Bluetooth mode.

As pointed out by Dan Carpenter, hid-microsoft directly relied on that
assumption as well.

Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")

Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
---
 drivers/hid/hid-microsoft.c | 2 +-
 net/bluetooth/hidp/core.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index 8b3a922bdad3..2cf83856f2e4 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -303,7 +303,7 @@ static void ms_ff_worker(struct work_struct *work)
 	r->magnitude[MAGNITUDE_WEAK] = ms->weak;     /* right actuator */
 
 	ret = hid_hw_output_report(hdev, (__u8 *)r, sizeof(*r));
-	if (ret)
+	if (ret < 0)
 		hid_warn(hdev, "failed to send FF report\n");
 }
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 8d889969ae7e..bef84b95e2c4 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -267,7 +267,7 @@ static int hidp_get_raw_report(struct hid_device *hid,
 	set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
 	data[0] = report_number;
 	ret = hidp_send_ctrl_message(session, report_type, data, 1);
-	if (ret)
+	if (ret < 0)
 		goto err;
 
 	/* Wait for the return of the report. The returned report
@@ -343,7 +343,7 @@ static int hidp_set_raw_report(struct hid_device *hid, unsigned char reportnum,
 	data[0] = reportnum;
 	set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
 	ret = hidp_send_ctrl_message(session, report_type, data, count);
-	if (ret)
+	if (ret < 0)
 		goto err;
 
 	/* Wait for the ACK from the device. */
-- 
2.23.0

Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

[Dan Carpenter]

On Fri, Sep 06, 2019 at 02:06:44PM +0300, Dan Elkouby wrote:
> hidp_send_message was changed to return non-zero values on success,
> which some other bits did not expect. This caused spurious errors to be
> propagated through the stack, breaking some drivers, such as hid-sony
> for the Dualshock 4 in Bluetooth mode.
> 
> As pointed out by Dan Carpenter, hid-microsoft directly relied on that
> assumption as well.
> 
> Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
> 
> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

[Jiri Kosina]

On Fri, 6 Sep 2019, Dan Elkouby wrote:

> hidp_send_message was changed to return non-zero values on success,
> which some other bits did not expect. This caused spurious errors to be
> propagated through the stack, breaking some drivers, such as hid-sony
> for the Dualshock 4 in Bluetooth mode.
> 
> As pointed out by Dan Carpenter, hid-microsoft directly relied on that
> assumption as well.
> 
> Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
> 
> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>

Reviewed-by: Jiri Kosina <jkosina@suse.cz>

Marcel, are you taking this through your tree?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

[Marcel Holtmann]

Hi Jiri,

>> hidp_send_message was changed to return non-zero values on success,
>> which some other bits did not expect. This caused spurious errors to be
>> propagated through the stack, breaking some drivers, such as hid-sony
>> for the Dualshock 4 in Bluetooth mode.
>> 
>> As pointed out by Dan Carpenter, hid-microsoft directly relied on that
>> assumption as well.
>> 
>> Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
>> 
>> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
> 
> Reviewed-by: Jiri Kosina <jkosina@suse.cz>
> 
> Marcel, are you taking this through your tree?

I am taking this through my tree. And yes, I applied the updated patch, but answered the other ;)

Regards

Marcel

Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

[Andrey Smirnov]

> I am taking this through my tree. And yes, I applied the updated patch, but answered the other ;)
>
> Regards
>
> Marcel

Are you also going to mark this patch for inclusion to stable tree? I haven't seen it in any of the stable queues (netdev, Greg's), so just wanted to check.

Thanks,
Andrey Smirnov