* [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
@ 2019-09-06 9:41 Dan Elkouby
2019-09-06 10:13 ` Dan Carpenter
2019-09-06 13:56 ` [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Marcel Holtmann
0 siblings, 2 replies; 13+ messages in thread
From: Dan Elkouby @ 2019-09-06 9:41 UTC (permalink / raw)
Cc: Dan Elkouby, Marcel Holtmann, Johan Hedberg, David S. Miller,
Dan Carpenter, Fabian Henneke, Brian Norris, Al Viro,
Andrea Parri, linux-bluetooth, netdev, linux-kernel
Commit 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return
number of queued bytes") changed hidp_send_message 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 (all?)
drivers, such as hid-sony for the Dualshock 4 in Bluetooth mode.
Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
2019-09-06 9:41 [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Dan Elkouby
@ 2019-09-06 10:13 ` Dan Carpenter
2019-09-06 10:40 ` Dan Elkouby
2019-09-06 11:06 ` [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message Dan Elkouby
2019-09-06 13:56 ` [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Marcel Holtmann
1 sibling, 2 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-09-06 10:13 UTC (permalink / raw)
To: Dan Elkouby
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
On Fri, Sep 06, 2019 at 12:41:57PM +0300, Dan Elkouby wrote:
> Commit 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return
> number of queued bytes") changed hidp_send_message 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 (all?)
> drivers, such as hid-sony for the Dualshock 4 in Bluetooth mode.
>
> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
I think we also need to update update ms_ff_worker() which assumes that
hid_hw_output_report() returns zero on success. Please use the Fixes
tag for this since a lot of scripts rely on it to decide what to
backport.
Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
Otherwise, it looks good. Thanks for catching this.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
2019-09-06 10:13 ` Dan Carpenter
@ 2019-09-06 10:40 ` Dan Elkouby
2019-09-06 10:58 ` Dan Carpenter
2019-09-06 11:06 ` [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message Dan Elkouby
1 sibling, 1 reply; 13+ messages in thread
From: Dan Elkouby @ 2019-09-06 10:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
On Fri, Sep 6, 2019 at 1:14 PM Dan Carpenter wrote:
> I think we also need to update update ms_ff_worker() which assumes that
> hid_hw_output_report() returns zero on success.
Yes, it looks like that's the case. Should I amend my patch to include
this fix, or should it be a separate patch? I don't have access to any
hardware covered by hid-microsoft, so I won't be able to test it.
> Please use the Fixes
> tag for this since a lot of scripts rely on it to decide what to
> backport.
>
> Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
Will do.
> Otherwise, it looks good. Thanks for catching this.
Thanks for taking a look!
(Sorry for sending this twice, I'm not used to mailing lists and forgot
to reply to all.)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
2019-09-06 10:40 ` Dan Elkouby
@ 2019-09-06 10:58 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-09-06 10:58 UTC (permalink / raw)
To: Dan Elkouby
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
On Fri, Sep 06, 2019 at 01:40:15PM +0300, Dan Elkouby wrote:
> On Fri, Sep 6, 2019 at 1:14 PM Dan Carpenter wrote:
> > I think we also need to update update ms_ff_worker() which assumes that
> > hid_hw_output_report() returns zero on success.
>
> Yes, it looks like that's the case. Should I amend my patch to include
> this fix, or should it be a separate patch? I don't have access to any
> hardware covered by hid-microsoft, so I won't be able to test it.
>
Yes. Please amend the patch. We all understand that you don't have
the hardware so it's not a problem. If you want to blame me in the
commit message that's fine. "Dan Carpenter pointed out a related issue
in ms_ff_worker()". But we're only silencing a warning so it can't
really break anything.
You can add my Reviewed-by tag as well when you resend.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
2019-09-06 10:13 ` Dan Carpenter
2019-09-06 10:40 ` Dan Elkouby
@ 2019-09-06 11:06 ` Dan Elkouby
2019-09-06 11:11 ` Dan Carpenter
2019-09-06 11:31 ` Jiri Kosina
1 sibling, 2 replies; 13+ messages in thread
From: Dan Elkouby @ 2019-09-06 11:06 UTC (permalink / raw)
To: Dan Carpenter
Cc: Dan Elkouby, Jiri Kosina, Benjamin Tissoires, Marcel Holtmann,
Johan Hedberg, David S. Miller, Brian Norris, Fabian Henneke,
Al Viro, Andrea Parri, linux-input, linux-kernel,
linux-bluetooth, netdev
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
2019-09-06 11:06 ` [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message Dan Elkouby
@ 2019-09-06 11:11 ` Dan Carpenter
2019-09-06 11:31 ` Jiri Kosina
1 sibling, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-09-06 11:11 UTC (permalink / raw)
To: Dan Elkouby
Cc: Jiri Kosina, Benjamin Tissoires, Marcel Holtmann, Johan Hedberg,
David S. Miller, Brian Norris, Fabian Henneke, Al Viro,
Andrea Parri, linux-input, linux-kernel, linux-bluetooth, netdev
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
2019-09-06 11:06 ` [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message Dan Elkouby
2019-09-06 11:11 ` Dan Carpenter
@ 2019-09-06 11:31 ` Jiri Kosina
2019-09-06 13:57 ` Marcel Holtmann
1 sibling, 1 reply; 13+ messages in thread
From: Jiri Kosina @ 2019-09-06 11:31 UTC (permalink / raw)
To: Dan Elkouby
Cc: Dan Carpenter, Benjamin Tissoires, Marcel Holtmann,
Johan Hedberg, David S. Miller, Brian Norris, Fabian Henneke,
Al Viro, Andrea Parri, linux-input, linux-kernel,
linux-bluetooth, netdev
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
2019-09-06 9:41 [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Dan Elkouby
2019-09-06 10:13 ` Dan Carpenter
@ 2019-09-06 13:56 ` Marcel Holtmann
2019-09-06 14:05 ` Dan Elkouby
2019-09-06 14:08 ` Dan Carpenter
1 sibling, 2 replies; 13+ messages in thread
From: Marcel Holtmann @ 2019-09-06 13:56 UTC (permalink / raw)
To: Dan Elkouby
Cc: Johan Hedberg, David S. Miller, Dan Carpenter, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
Hi Dan,
> Commit 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return
> number of queued bytes") changed hidp_send_message 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 (all?)
> drivers, such as hid-sony for the Dualshock 4 in Bluetooth mode.
>
> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
> ---
> net/bluetooth/hidp/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
2019-09-06 11:31 ` Jiri Kosina
@ 2019-09-06 13:57 ` Marcel Holtmann
2019-09-23 20:33 ` Andrey Smirnov
0 siblings, 1 reply; 13+ messages in thread
From: Marcel Holtmann @ 2019-09-06 13:57 UTC (permalink / raw)
To: Jiri Kosina
Cc: Dan Elkouby, Dan Carpenter, Benjamin Tissoires, Johan Hedberg,
David S. Miller, Brian Norris, Fabian Henneke, Al Viro,
Andrea Parri, linux-input, linux-kernel, linux-bluetooth, netdev
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
2019-09-06 13:56 ` [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Marcel Holtmann
@ 2019-09-06 14:05 ` Dan Elkouby
2019-09-06 14:08 ` Dan Carpenter
1 sibling, 0 replies; 13+ messages in thread
From: Dan Elkouby @ 2019-09-06 14:05 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Johan Hedberg, David S. Miller, Dan Carpenter, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
On Fri, Sep 6, 2019 at 4:56 PM Marcel Holtmann <marcel@holtmann.org> wrote:
> patch has been applied to bluetooth-next tree.
>
Thanks a lot!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
2019-09-06 13:56 ` [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Marcel Holtmann
2019-09-06 14:05 ` Dan Elkouby
@ 2019-09-06 14:08 ` Dan Carpenter
2019-09-06 14:12 ` Marcel Holtmann
1 sibling, 1 reply; 13+ messages in thread
From: Dan Carpenter @ 2019-09-06 14:08 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dan Elkouby, Johan Hedberg, David S. Miller, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
On Fri, Sep 06, 2019 at 03:56:29PM +0200, Marcel Holtmann wrote:
> Hi Dan,
>
> > Commit 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return
> > number of queued bytes") changed hidp_send_message 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 (all?)
> > drivers, such as hid-sony for the Dualshock 4 in Bluetooth mode.
> >
> > Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
> > ---
> > net/bluetooth/hidp/core.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> patch has been applied to bluetooth-next tree.
>
The v2 added an additional fix and used the Fixes tag. Could you apply
that instead?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
2019-09-06 14:08 ` Dan Carpenter
@ 2019-09-06 14:12 ` Marcel Holtmann
0 siblings, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2019-09-06 14:12 UTC (permalink / raw)
To: Dan Carpenter
Cc: Dan Elkouby, Johan Hedberg, David S. Miller, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
Hi Dan,
>>> Commit 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return
>>> number of queued bytes") changed hidp_send_message 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 (all?)
>>> drivers, such as hid-sony for the Dualshock 4 in Bluetooth mode.
>>>
>>> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
>>> ---
>>> net/bluetooth/hidp/core.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> patch has been applied to bluetooth-next tree.
>>
>
> The v2 added an additional fix and used the Fixes tag. Could you apply
> that instead?
see my reply to Jiri. I replied to the wrong patch, but actually applied to the updated one.
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
2019-09-06 13:57 ` Marcel Holtmann
@ 2019-09-23 20:33 ` Andrey Smirnov
0 siblings, 0 replies; 13+ messages in thread
From: Andrey Smirnov @ 2019-09-23 20:33 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Andrey Smirnov, Pierre-Loup A . Griffais, linux-input,
linux-kernel, linux-bluetooth, netdev
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-09-23 20:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 9:41 [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Dan Elkouby
2019-09-06 10:13 ` Dan Carpenter
2019-09-06 10:40 ` Dan Elkouby
2019-09-06 10:58 ` Dan Carpenter
2019-09-06 11:06 ` [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message Dan Elkouby
2019-09-06 11:11 ` Dan Carpenter
2019-09-06 11:31 ` Jiri Kosina
2019-09-06 13:57 ` Marcel Holtmann
2019-09-23 20:33 ` Andrey Smirnov
2019-09-06 13:56 ` [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report Marcel Holtmann
2019-09-06 14:05 ` Dan Elkouby
2019-09-06 14:08 ` Dan Carpenter
2019-09-06 14:12 ` Marcel Holtmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).