linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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	[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	[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).