tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

avoid 32-bit overflow in HID report-size computation (sys/dev/hid/hid.c)



Hi,

Auditing the HID report-descriptor parser I noticed that hid_report_size()
and the position accumulation in hid_get_item() compute
loc.pos + loc.size * loc.count with uint32_t operands taken straight from
the report descriptor, with no bound.  A crafted descriptor can wrap the
32-bit multiply, so hid_report_size() returns a wrong (usually too small)
report size.

I don't think it is exploitable: hid_get_data() clamps reads to the
caller's buffer and the drivers reject reports larger than the endpoint.
But the length is still wrong, so it seems worth doing the arithmetic in
64-bit and saturating.

Caveat: I build-tested only the equivalent FreeBSD parser, which is the
same code; I have not built a NetBSD kernel with this, so please treat it
as a proposal.  For reference the same fix is under review in FreeBSD as
https://reviews.freebsd.org/D60846 .

Does this look reasonable?

--- a/sys/dev/hid/hid.c	2026-10-18 23:21:18.340040000 +0000
+++ b/sys/dev/hid/hid.c	2026-10-18 23:22:02.659433000 +0000
@@ -238,12 +238,16 @@
 					}
 					goto top;
 				} else {
+					uint64_t pos;
+
 					if (s->minset)
 						c->usage = c->usage_minimum;
 					*h = *c;
 					h->next = NULL;
-					c->loc.pos +=
-					    c->loc.size * c->loc.count;
+					pos = (uint64_t)c->loc.pos +
+					    (uint64_t)c->loc.size * c->loc.count;
+					c->loc.pos = pos > 0xffffffffU ?
+					    0xffffffffU : (uint32_t)pos;
 					s->minset = 0;
 					s->nu = 0;
 					hid_clear_local(c);
@@ -385,7 +389,7 @@
 {
 	struct hid_data *d;
 	struct hid_item h;
-	int lo, hi;
+	int64_t lo, hi;
 
 	h.report_ID = 0;
 	lo = hi = -1;
@@ -405,8 +409,11 @@
 				}
 #endif
 			}
-			hi = h.loc.pos + h.loc.size * h.loc.count;
-			DPRINTFN(2,("hid_report_size: lo=%d hi=%d\n", lo, hi));
+			hi = (uint64_t)h.loc.pos +
+			    (uint64_t)h.loc.size * h.loc.count;
+			if (hi > 0xffffffffU)
+				hi = 0xffffffffU;
+			DPRINTFN(2,("hid_report_size: lo=%d hi=%d\n", (int)lo, (int)hi));
 		}
 	}
 	hid_end_parse(d);


Thanks,
Osvaldo Janeri Filho



Home | Main Index | Thread Index | Old Index