[PATCH] HID: logitech-hidpp: Check maxfield in hidpp_get_report_length()

Günther Noack posted 1 patch 3 months, 1 week ago
There is a newer version of this series
drivers/hid/hid-logitech-hidpp.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] HID: logitech-hidpp: Check maxfield in hidpp_get_report_length()
Posted by Günther Noack 3 months, 1 week ago
Do not crash when a report has no fields.

Fake USB gadgets can send their own HID report descriptors and can define report
structures without valid fields.  This can be used to crash the kernel over USB.

Cc: stable@vger.kernel.org
Signed-off-by: Günther Noack <gnoack@google.com>
---
 drivers/hid/hid-logitech-hidpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 9ced0e4d46ae..919ba9f50292 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4316,6 +4316,9 @@ static int hidpp_get_report_length(struct hid_device *hdev, int id)
 	if (!report)
 		return 0;
 
+	if (!report->maxfield)
+		return 0;
+
 	return report->field[0]->report_count + 1;
 }
 
-- 
2.52.0.457.g6b5491de43-goog
Re: [PATCH] HID: logitech-hidpp: Check maxfield in hidpp_get_report_length()
Posted by Greg KH 3 months, 1 week ago
On Fri, Jan 09, 2026 at 11:59:12AM +0100, Günther Noack wrote:
> Do not crash when a report has no fields.
> 
> Fake USB gadgets can send their own HID report descriptors and can define report
> structures without valid fields.  This can be used to crash the kernel over USB.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 9ced0e4d46ae..919ba9f50292 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4316,6 +4316,9 @@ static int hidpp_get_report_length(struct hid_device *hdev, int id)
>  	if (!report)
>  		return 0;
>  
> +	if (!report->maxfield)
> +		return 0;

Combine this with the if() above this?

And if we are going to be handling "malicious" USB devices, be careful,
you are just moving the target lower down, you also need to audit ALL
data coming from the device, not just the descriptors.  I'm all for
this, just realize that this is a change in how Linux treats devices
(and all other operating systems as well.)

For now, we strongly recommend not allowing "untrusted" devices to bind
to your system if this is a threat model you care about.

Not to reject this, or your other patch like this, just letting you
know.

thanks,

greg k-h
Re: [PATCH] HID: logitech-hidpp: Check maxfield in hidpp_get_report_length()
Posted by Günther Noack 3 months, 1 week ago
On Fri, Jan 09, 2026 at 12:14:43PM +0100, Greg KH wrote:
> On Fri, Jan 09, 2026 at 11:59:12AM +0100, Günther Noack wrote:
> > Do not crash when a report has no fields.
> > 
> > Fake USB gadgets can send their own HID report descriptors and can define report
> > structures without valid fields.  This can be used to crash the kernel over USB.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Günther Noack <gnoack@google.com>
> > ---
> >  drivers/hid/hid-logitech-hidpp.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> > index 9ced0e4d46ae..919ba9f50292 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -4316,6 +4316,9 @@ static int hidpp_get_report_length(struct hid_device *hdev, int id)
> >  	if (!report)
> >  		return 0;
> >  
> > +	if (!report->maxfield)
> > +		return 0;
> 
> Combine this with the if() above this?

OK, done. I sent a V2:
https://lore.kernel.org/all/20260109122557.3166556-3-gnoack@google.com/


> And if we are going to be handling "malicious" USB devices, be careful,
> you are just moving the target lower down, you also need to audit ALL
> data coming from the device, not just the descriptors.  I'm all for
> this, just realize that this is a change in how Linux treats devices
> (and all other operating systems as well.)

Thanks.  Yes, I realize that the later communication with the device is also a
potential way to trigger bugs.


> For now, we strongly recommend not allowing "untrusted" devices to bind
> to your system if this is a threat model you care about.
> 
> Not to reject this, or your other patch like this, just letting you
> know.

Acknowledged, thanks.

-Günther