Message ID | 20191029162555.426-2-kamal@canonical.com |
---|---|
State | New |
Headers | show |
Series | [Eoan,Disco,1/1] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message | expand |
On 29.10.19 17:25, Kamal Mostafa wrote: > From: Dan Elkouby <streetwalkermc@gmail.com> > > 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 <streetwalkermc@gmail.com> > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > Reviewed-by: Jiri Kosina <jkosina@suse.cz> > Signed-off-by: Marcel Holtmann <marcel@holtmann.org> > (cherry picked from commit 8bb3537095f107ed55ad51f6241165b397aaafac) > Signed-off-by: Kamal Mostafa <kamal@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- 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. */ >
On 10/29/19 9:25 AM, Kamal Mostafa wrote: > From: Dan Elkouby <streetwalkermc@gmail.com> > > 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 <streetwalkermc@gmail.com> > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > Reviewed-by: Jiri Kosina <jkosina@suse.cz> > Signed-off-by: Marcel Holtmann <marcel@holtmann.org> > (cherry picked from commit 8bb3537095f107ed55ad51f6241165b397aaafac) > Signed-off-by: Kamal Mostafa <kamal@canonical.com> Acked-by: Connor Kuehl <connor.kuehl@canonical.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. */ >
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. */