Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] HID: u2fzero: fix general protection fault in u2fzero_recv
@ 2026-04-21 13:48 l1za0.sec
  2026-05-21 14:27 ` Benjamin Tissoires
  0 siblings, 1 reply; 2+ messages in thread
From: l1za0.sec @ 2026-04-21 13:48 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel

From: Haocheng Yu <l1za0.sec@gmail.com>

A general protection fault in u2fzero_recv is reported by a
modified Syzkaller-based kernel fuzzing tool we developed.

The cause is that u2fzero_probe() calls the u2fzero_fill_in_urb()
function but ignores its return value. When the urb setting fails,
dev->urb remains NULL, but u2fzero_probe() continues to run. When
`dev->urb->context = &ctx;` in u2fzero_recv() is executed, the
KASAN null pointer dereference crash will occur.

To fix this vulnerability, I added a check for the return value of
u2fzero_fill_in_urb() and aborted u2fzero_probe() on error. And I
added a NULL value check for dev->urb in u2fzero_recv() to further
ensure its integrity.

Signed-off-by: Haocheng Yu <l1za0.sec@gmail.com>
---
 drivers/hid/hid-u2fzero.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
index 744a91e6e78c..c51f6dd80635 100644
--- a/drivers/hid/hid-u2fzero.c
+++ b/drivers/hid/hid-u2fzero.c
@@ -134,6 +134,12 @@ static int u2fzero_recv(struct u2fzero_device *dev,
 
 	memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
 
+	if (!dev->urb) {
+		hid_err(hdev, "recv called without initialized URB");
+		ret = -ENODEV;
+		goto err;
+	}
+
 	dev->urb->context = &ctx;
 	init_completion(&ctx.done);
 
@@ -341,7 +347,11 @@ static int u2fzero_probe(struct hid_device *hdev,
 	if (ret)
 		return ret;
 
-	u2fzero_fill_in_urb(dev);
+	ret = u2fzero_fill_in_urb(dev);
+	if (ret) {
+		hid_hw_stop(hdev);
+		return ret;
+	}
 
 	dev->present = true;
 

base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] HID: u2fzero: fix general protection fault in u2fzero_recv
  2026-04-21 13:48 [PATCH] HID: u2fzero: fix general protection fault in u2fzero_recv l1za0.sec
@ 2026-05-21 14:27 ` Benjamin Tissoires
  0 siblings, 0 replies; 2+ messages in thread
From: Benjamin Tissoires @ 2026-05-21 14:27 UTC (permalink / raw)
  To: l1za0.sec; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel


Hi,

Thanks for the patch and sorry for the delay.

On Apr 21 2026, l1za0.sec@gmail.com wrote:
> From: Haocheng Yu <l1za0.sec@gmail.com>
> 
> A general protection fault in u2fzero_recv is reported by a
> modified Syzkaller-based kernel fuzzing tool we developed.
> 
> The cause is that u2fzero_probe() calls the u2fzero_fill_in_urb()
> function but ignores its return value. When the urb setting fails,
> dev->urb remains NULL, but u2fzero_probe() continues to run. When
> `dev->urb->context = &ctx;` in u2fzero_recv() is executed, the
> KASAN null pointer dereference crash will occur.
> 
> To fix this vulnerability, I added a check for the return value of
> u2fzero_fill_in_urb() and aborted u2fzero_probe() on error. And I
> added a NULL value check for dev->urb in u2fzero_recv() to further
> ensure its integrity.

Couple of things:
- sorry, but a couple of days later someone else send a patch to tackle
  the same problem at a better level IMO:
  https://lore.kernel.org/linux-input/20260424-u2fzero-probe-urb-unwind-v1-1-mhun512@gmail.com/
  So I'm going to take this one instead of yours. I would appreciate if
  you can confirm this fixes your current reproducer.

- Your commit description is not great. Please have a look at
  Documentation/process/submitting-patches.rst The thing that made me
  notice this is the non imperative form (quoting
  submitting-patches.rst):
  """
  Describe your changes in imperative mood,
  e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do
  frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders
  to the codebase to change its behaviour.
  """
  The paragraph above is also hard to follow but valuable for
  understanding why it needs to be fixed.

Again, in the end the problem is that is dev->urb is NULL, there are
little chances the system is healthy and the device will work correctly,
so it's best to bail out early in probe like the other patch I
mentioned.

Cheers,
Benjamin

> 
> Signed-off-by: Haocheng Yu <l1za0.sec@gmail.com>
> ---
>  drivers/hid/hid-u2fzero.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
> index 744a91e6e78c..c51f6dd80635 100644
> --- a/drivers/hid/hid-u2fzero.c
> +++ b/drivers/hid/hid-u2fzero.c
> @@ -134,6 +134,12 @@ static int u2fzero_recv(struct u2fzero_device *dev,
>  
>  	memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
>  
> +	if (!dev->urb) {
> +		hid_err(hdev, "recv called without initialized URB");
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
>  	dev->urb->context = &ctx;
>  	init_completion(&ctx.done);
>  
> @@ -341,7 +347,11 @@ static int u2fzero_probe(struct hid_device *hdev,
>  	if (ret)
>  		return ret;
>  
> -	u2fzero_fill_in_urb(dev);
> +	ret = u2fzero_fill_in_urb(dev);
> +	if (ret) {
> +		hid_hw_stop(hdev);
> +		return ret;
> +	}
>  
>  	dev->present = true;
>  
> 
> base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
> -- 
> 2.51.0
> 
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-21 14:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 13:48 [PATCH] HID: u2fzero: fix general protection fault in u2fzero_recv l1za0.sec
2026-05-21 14:27 ` Benjamin Tissoires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox