* [PATCH 1/2] HID: u2fzero: explicitly check for errors
@ 2021-10-18 6:47 Andrej Shadura
2021-10-18 6:48 ` [PATCH 2/2] HID: u2fzero: properly handle timeouts in usb_submit_urb Andrej Shadura
2021-10-18 6:56 ` [PATCH 1/2] HID: u2fzero: explicitly check for errors Greg KH
0 siblings, 2 replies; 5+ messages in thread
From: Andrej Shadura @ 2021-10-18 6:47 UTC (permalink / raw)
To: Jiří Kosina; +Cc: linux-input, linux-usb, kernel, Benjamin Tissoires
The previous commit fixed handling of incomplete packets but broke error
handling: offsetof returns an unsigned value (size_t), but when compared
against the signed return value, the return value is interpreted as if
it were unsigned, so negative return values are never less than the
offset.
Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without data")
Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
drivers/hid/hid-u2fzero.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
index d70cd3d7f583..5145d758bea0 100644
--- a/drivers/hid/hid-u2fzero.c
+++ b/drivers/hid/hid-u2fzero.c
@@ -200,7 +200,7 @@ static int u2fzero_rng_read(struct hwrng *rng, void *data,
ret = u2fzero_recv(dev, &req, &resp);
/* ignore errors or packets without data */
- if (ret < offsetof(struct u2f_hid_msg, init.data))
+ if (ret < 0 || ret < offsetof(struct u2f_hid_msg, init.data))
return 0;
/* only take the minimum amount of data it is safe to take */
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] HID: u2fzero: properly handle timeouts in usb_submit_urb
2021-10-18 6:47 [PATCH 1/2] HID: u2fzero: explicitly check for errors Andrej Shadura
@ 2021-10-18 6:48 ` Andrej Shadura
2021-10-18 6:56 ` [PATCH 1/2] HID: u2fzero: explicitly check for errors Greg KH
1 sibling, 0 replies; 5+ messages in thread
From: Andrej Shadura @ 2021-10-18 6:48 UTC (permalink / raw)
To: Jiří Kosina; +Cc: linux-input, linux-usb, kernel, Benjamin Tissoires
The wait_for_completion_timeout function returns 0 if timed out or a
positive value if completed. Hence, "less than zero" comparison always
misses timeouts and doesn't kill the URB as it should, leading to
re-sending it while it is active.
Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
drivers/hid/hid-u2fzero.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
index 5145d758bea0..562da98cfb82 100644
--- a/drivers/hid/hid-u2fzero.c
+++ b/drivers/hid/hid-u2fzero.c
@@ -132,7 +132,7 @@ static int u2fzero_recv(struct u2fzero_device *dev,
ret = (wait_for_completion_timeout(
&ctx.done, msecs_to_jiffies(USB_CTRL_SET_TIMEOUT)));
- if (ret < 0) {
+ if (ret == 0) {
usb_kill_urb(dev->urb);
hid_err(hdev, "urb submission timed out");
} else {
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] HID: u2fzero: explicitly check for errors
2021-10-18 6:47 [PATCH 1/2] HID: u2fzero: explicitly check for errors Andrej Shadura
2021-10-18 6:48 ` [PATCH 2/2] HID: u2fzero: properly handle timeouts in usb_submit_urb Andrej Shadura
@ 2021-10-18 6:56 ` Greg KH
2021-10-18 12:06 ` Andrej Shadura
1 sibling, 1 reply; 5+ messages in thread
From: Greg KH @ 2021-10-18 6:56 UTC (permalink / raw)
To: Andrej Shadura
Cc: Jiří Kosina, linux-input, linux-usb, kernel,
Benjamin Tissoires
On Mon, Oct 18, 2021 at 08:47:59AM +0200, Andrej Shadura wrote:
> The previous commit fixed handling of incomplete packets but broke error
> handling: offsetof returns an unsigned value (size_t), but when compared
> against the signed return value, the return value is interpreted as if
> it were unsigned, so negative return values are never less than the
> offset.
>
> Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without data")
> Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
Nit, you need a ' ' before the '(' character here, and you only need 12
digits of the sha1. It should look like:
Fixes: 22d65765f211 ("HID: u2fzero: ignore incomplete packets without data")
Also, how about a cc: stable for these as well?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] HID: u2fzero: explicitly check for errors
2021-10-18 6:56 ` [PATCH 1/2] HID: u2fzero: explicitly check for errors Greg KH
@ 2021-10-18 12:06 ` Andrej Shadura
2021-10-18 12:09 ` Andrej Shadura
0 siblings, 1 reply; 5+ messages in thread
From: Andrej Shadura @ 2021-10-18 12:06 UTC (permalink / raw)
To: Greg KH
Cc: Jiří Kosina, linux-input, linux-usb, kernel,
Benjamin Tissoires
On 18/10/2021 08:56, Greg KH wrote:
> On Mon, Oct 18, 2021 at 08:47:59AM +0200, Andrej Shadura wrote:
>> The previous commit fixed handling of incomplete packets but broke error
>> handling: offsetof returns an unsigned value (size_t), but when compared
>> against the signed return value, the return value is interpreted as if
>> it were unsigned, so negative return values are never less than the
>> offset.
>>
>> Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without data")
>> Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
>
> Nit, you need a ' ' before the '(' character here, and you only need 12
> digits of the sha1. It should look like:
> Fixes: 22d65765f211 ("HID: u2fzero: ignore incomplete packets without data")
Thanks, is there a script or something to help generate those? (I’m
surprised --fixup cannot generate them.)
> Also, how about a cc: stable for these as well?
Sure, will do.
--
Cheers,
Andrej
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] HID: u2fzero: explicitly check for errors
2021-10-18 12:06 ` Andrej Shadura
@ 2021-10-18 12:09 ` Andrej Shadura
0 siblings, 0 replies; 5+ messages in thread
From: Andrej Shadura @ 2021-10-18 12:09 UTC (permalink / raw)
To: Greg KH
Cc: Jiří Kosina, linux-input, linux-usb, kernel,
Benjamin Tissoires
On 18/10/2021 14:06, Andrej Shadura wrote:
> On 18/10/2021 08:56, Greg KH wrote:
>> On Mon, Oct 18, 2021 at 08:47:59AM +0200, Andrej Shadura wrote:
>>> The previous commit fixed handling of incomplete packets but broke error
>>> handling: offsetof returns an unsigned value (size_t), but when compared
>>> against the signed return value, the return value is interpreted as if
>>> it were unsigned, so negative return values are never less than the
>>> offset.
>>>
>>> Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without
>>> data")
>>> Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and
>>> RNG")
>>
>> Nit, you need a ' ' before the '(' character here, and you only need 12
>> digits of the sha1. It should look like:
>> Fixes: 22d65765f211 ("HID: u2fzero: ignore incomplete packets
>> without data")
>
> Thanks, is there a script or something to help generate those? (I’m
> surprised --fixup cannot generate them.)
Replying to myself, apparently the answer to this is now in the
"submitting patches" document.
--
Cheers,
Andrej
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-18 12:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 6:47 [PATCH 1/2] HID: u2fzero: explicitly check for errors Andrej Shadura
2021-10-18 6:48 ` [PATCH 2/2] HID: u2fzero: properly handle timeouts in usb_submit_urb Andrej Shadura
2021-10-18 6:56 ` [PATCH 1/2] HID: u2fzero: explicitly check for errors Greg KH
2021-10-18 12:06 ` Andrej Shadura
2021-10-18 12:09 ` Andrej Shadura
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).