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