[Eoan,Disco][PATCH 0/1] Bluetooth hidp_send_message fix

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[Eoan,Disco][PATCH 0/1] Bluetooth hidp_send_message fix

Kamal Mostafa-2
BugLink: https://bugs.launchpad.net/bugs/1850443

Clean cherry-pick from mainline, fixes a hidp bug which can manifest
in Eoan.  Also applies to Disco as a good safety measure.

 -Kamal

-----

Dan Elkouby (1):
  Bluetooth: hidp: Fix assumptions on the return value of
    hidp_send_message

 drivers/hid/hid-microsoft.c | 2 +-
 net/bluetooth/hidp/core.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Eoan, Disco][PATCH 1/1] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

Kamal Mostafa-2
From: Dan Elkouby <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1850443

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 <[hidden email]>
Reviewed-by: Dan Carpenter <[hidden email]>
Reviewed-by: Jiri Kosina <[hidden email]>
Signed-off-by: Marcel Holtmann <[hidden email]>
(cherry picked from commit 8bb3537095f107ed55ad51f6241165b397aaafac)
Signed-off-by: Kamal Mostafa <[hidden email]>
---
 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.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [Eoan, Disco][PATCH 1/1] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

Stefan Bader-2
On 29.10.19 17:25, Kamal Mostafa wrote:

> From: Dan Elkouby <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1850443
>
> 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 <[hidden email]>
> Reviewed-by: Dan Carpenter <[hidden email]>
> Reviewed-by: Jiri Kosina <[hidden email]>
> Signed-off-by: Marcel Holtmann <[hidden email]>
> (cherry picked from commit 8bb3537095f107ed55ad51f6241165b397aaafac)
> Signed-off-by: Kamal Mostafa <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>
> ---

Bug report should have proper SRU justification in the description.

-Stefan

>  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. */
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [Eoan, Disco][PATCH 1/1] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message

Connor Kuehl
In reply to this post by Kamal Mostafa-2
On 10/29/19 9:25 AM, Kamal Mostafa wrote:

> From: Dan Elkouby <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1850443
>
> 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 <[hidden email]>
> Reviewed-by: Dan Carpenter <[hidden email]>
> Reviewed-by: Jiri Kosina <[hidden email]>
> Signed-off-by: Marcel Holtmann <[hidden email]>
> (cherry picked from commit 8bb3537095f107ed55ad51f6241165b397aaafac)
> Signed-off-by: Kamal Mostafa <[hidden email]>

Acked-by: Connor Kuehl <[hidden email]>

> ---
>   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. */
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [Eoan,Disco][PATCH 0/1] Bluetooth hidp_send_message fix

Khaled Elmously
In reply to this post by Kamal Mostafa-2
On 2019-10-29 09:25:54 , Kamal Mostafa wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850443
>
> Clean cherry-pick from mainline, fixes a hidp bug which can manifest
> in Eoan.  Also applies to Disco as a good safety measure.
>
>  -Kamal
>
> -----
>
> Dan Elkouby (1):
>   Bluetooth: hidp: Fix assumptions on the return value of
>     hidp_send_message
>
>  drivers/hid/hid-microsoft.c | 2 +-
>  net/bluetooth/hidp/core.c   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team