From: Miod Vallat Subject: signedness fixes in hid(4) To: tech@openbsd.org Date: Sun, 4 May 2025 19:48:40 +0000 This is NetBSD PR#53605 via FreeBSD: https://cgit.freebsd.org/src/commit/?id=38b67578fb4bbf568f7012ca3921a4d15cfe7c5d It looks like some devices have report descriptors which causes us to compute negative array offsets due to sign extension. Please test and report any regression. Index: sys/dev/hid/hid.c =================================================================== RCS file: /OpenBSD/src/sys/dev/hid/hid.c,v diff -u -p -u -p -r1.7 hid.c --- sys/dev/hid/hid.c 10 May 2024 10:49:10 -0000 1.7 +++ sys/dev/hid/hid.c 4 May 2025 19:37:42 -0000 @@ -49,7 +49,7 @@ #define MAXID 16 struct hid_pos_data { - int32_t rid; + uint32_t rid; uint32_t pos; }; @@ -59,9 +59,9 @@ struct hid_data { const uint8_t *p; struct hid_item cur[MAXPUSH]; struct hid_pos_data last_pos[MAXID]; - int32_t usages_min[MAXUSAGE]; - int32_t usages_max[MAXUSAGE]; - int32_t usage_last; /* last seen usage */ + uint32_t usages_min[MAXUSAGE]; + uint32_t usages_max[MAXUSAGE]; + uint32_t usage_last; /* last seen usage */ uint32_t loc_size; /* last seen size */ uint32_t loc_count; /* last seen count */ enum hid_kind kind; @@ -92,7 +92,7 @@ hid_clear_local(struct hid_item *c) } static void -hid_switch_rid(struct hid_data *s, struct hid_item *c, int32_t nextid) +hid_switch_rid(struct hid_data *s, struct hid_item *c, uint32_t nextid) { uint8_t i; @@ -195,6 +195,7 @@ hid_get_item(struct hid_data *s, struct uint32_t oldpos; int32_t mask; int32_t dval; + uint32_t uval; if (s == NULL) return (0); @@ -211,10 +212,10 @@ hid_get_item(struct hid_data *s, struct if (s->icount < s->ncount) { /* get current usage */ if (s->iusage < s->nusage) { - dval = s->usages_min[s->iusage] + s->ousage; - c->usage = dval; - s->usage_last = dval; - if (dval == s->usages_max[s->iusage]) { + uval = s->usages_min[s->iusage] + s->ousage; + c->usage = uval; + s->usage_last = uval; + if (uval == s->usages_max[s->iusage]) { s->iusage ++; s->ousage = 0; } else { @@ -222,7 +223,7 @@ hid_get_item(struct hid_data *s, struct } } else { DPRINTF("Using last usage\n"); - dval = s->usage_last; + uval = s->usage_last; } s->icount ++; /* @@ -267,29 +268,34 @@ hid_get_item(struct hid_data *s, struct } switch (bSize) { case 0: - dval = 0; + uval = 0; + dval = uval; mask = 0; break; case 1: - dval = hid_get_byte(s, 1); + uval = hid_get_byte(s, 1); + dval = (int8_t)uval; mask = 0xFF; break; case 2: - dval = hid_get_byte(s, 1); - dval |= hid_get_byte(s, 1) << 8; + uval = hid_get_byte(s, 1); + uval |= hid_get_byte(s, 1) << 8; + dval = (int16_t)uval; mask = 0xFFFF; break; case 4: - dval = hid_get_byte(s, 1); - dval |= hid_get_byte(s, 1) << 8; - dval |= hid_get_byte(s, 1) << 16; - dval |= hid_get_byte(s, 1) << 24; + uval = hid_get_byte(s, 1); + uval |= hid_get_byte(s, 1) << 8; + uval |= hid_get_byte(s, 1) << 16; + uval |= hid_get_byte(s, 1) << 24; + dval = uval; mask = 0xFFFFFFFF; break; default: - dval = hid_get_byte(s, bSize); + uval = hid_get_byte(s, bSize); + dval = uval; DPRINTF("bad length %u (data=0x%02x)\n", - bSize, dval); + bSize, uval); continue; } @@ -300,7 +306,7 @@ hid_get_item(struct hid_data *s, struct switch (bTag) { case 8: /* Input */ c->kind = hid_input; - c->flags = dval; + c->flags = uval; ret: c->loc.count = s->loc_count; c->loc.size = s->loc_size; @@ -326,11 +332,11 @@ hid_get_item(struct hid_data *s, struct case 9: /* Output */ c->kind = hid_output; - c->flags = dval; + c->flags = uval; goto ret; case 10: /* Collection */ c->kind = hid_collection; - c->collection = dval; + c->collection = uval; c->collevel++; c->usage = s->usage_last; *h = *c; @@ -356,7 +362,7 @@ hid_get_item(struct hid_data *s, struct case 1: /* Global */ switch (bTag) { case 0: - c->_usage_page = dval << 16; + c->_usage_page = uval << 16; break; case 1: c->logical_minimum = dval; @@ -371,21 +377,21 @@ hid_get_item(struct hid_data *s, struct c->physical_maximum = dval; break; case 5: - c->unit_exponent = dval; + c->unit_exponent = uval; break; case 6: - c->unit = dval; + c->unit = uval; break; case 7: /* mask because value is unsigned */ - s->loc_size = dval & mask; + s->loc_size = uval & mask; break; case 8: - hid_switch_rid(s, c, dval & mask); + hid_switch_rid(s, c, uval & mask); break; case 9: /* mask because value is unsigned */ - s->loc_count = dval & mask; + s->loc_count = uval & mask; break; case 10: /* Push */ if (s->pushlevel < MAXPUSH - 1) { @@ -428,14 +434,14 @@ hid_get_item(struct hid_data *s, struct switch (bTag) { case 0: if (bSize != 4) - dval = (dval & mask) | c->_usage_page; + uval = (uval & mask) | c->_usage_page; /* set last usage, in case of a collection */ - s->usage_last = dval; + s->usage_last = uval; if (s->nusage < MAXUSAGE) { - s->usages_min[s->nusage] = dval; - s->usages_max[s->nusage] = dval; + s->usages_min[s->nusage] = uval; + s->usages_max[s->nusage] = uval; s->nusage ++; } else { DPRINTF("max usage reached\n"); @@ -448,16 +454,16 @@ hid_get_item(struct hid_data *s, struct s->susage |= 1; if (bSize != 4) - dval = (dval & mask) | c->_usage_page; - c->usage_minimum = dval; + uval = (uval & mask) | c->_usage_page; + c->usage_minimum = uval; goto check_set; case 2: s->susage |= 2; if (bSize != 4) - dval = (dval & mask) | c->_usage_page; - c->usage_maximum = dval; + uval = (uval & mask) | c->_usage_page; + c->usage_maximum = uval; check_set: if (s->susage != 3) @@ -478,25 +484,25 @@ hid_get_item(struct hid_data *s, struct s->susage = 0; break; case 3: - c->designator_index = dval; + c->designator_index = uval; break; case 4: - c->designator_minimum = dval; + c->designator_minimum = uval; break; case 5: - c->designator_maximum = dval; + c->designator_maximum = uval; break; case 7: - c->string_index = dval; + c->string_index = uval; break; case 8: - c->string_minimum = dval; + c->string_minimum = uval; break; case 9: - c->string_maximum = dval; + c->string_maximum = uval; break; case 10: - c->set_delimiter = dval; + c->set_delimiter = uval; break; default: DPRINTF("Local bTag=%d\n", bTag); Index: sys/dev/hid/hid.h =================================================================== RCS file: /OpenBSD/src/sys/dev/hid/hid.h,v diff -u -p -u -p -r1.11 hid.h --- sys/dev/hid/hid.h 12 Aug 2023 20:47:06 -0000 1.11 +++ sys/dev/hid/hid.h 4 May 2025 19:37:42 -0000 @@ -54,27 +54,27 @@ struct hid_location { struct hid_item { /* Global */ - int32_t _usage_page; + uint32_t _usage_page; int32_t logical_minimum; int32_t logical_maximum; int32_t physical_minimum; int32_t physical_maximum; - int32_t unit_exponent; - int32_t unit; - int32_t report_ID; + uint32_t unit_exponent; + uint32_t unit; + uint32_t report_ID; /* Local */ - int32_t usage; - int32_t usage_minimum; - int32_t usage_maximum; - int32_t designator_index; - int32_t designator_minimum; - int32_t designator_maximum; - int32_t string_index; - int32_t string_minimum; - int32_t string_maximum; - int32_t set_delimiter; + uint32_t usage; + uint32_t usage_minimum; + uint32_t usage_maximum; + uint32_t designator_index; + uint32_t designator_minimum; + uint32_t designator_maximum; + uint32_t string_index; + uint32_t string_minimum; + uint32_t string_maximum; + uint32_t set_delimiter; /* Misc */ - int32_t collection; + uint32_t collection; int collevel; enum hid_kind kind; u_int32_t flags;