Download raw body.
signedness fixes in hid(4)
I didn't get a definitive answer on whether we want this when I sent
it months ago but it is needed for certain devices.
ok?
commit e6433115cb58faed8b4b68507d39c984e798a5b9
Author: joshua stein <jcs@jcs.org>
Date: Thu May 1 13:32:30 2025 -0500
hid: properly handle signed values
We were formerly parsing descriptor data like this:
0x15, 0x81, // Logical Minimum (129)
0x25, 0x7F, // Logical Maximum (127)
when it should be a signed value:
0x15, 0x81, // Logical Minimum (-127)
0x25, 0x7F, // Logical Maximum (127)
This caused issues comparing values like x/y min/max.
NetBSD made a similar change in 2018
diff --git sys/dev/hid/hid.c sys/dev/hid/hid.c
index e6203a7483f..8ed667ba5ac 100644
--- sys/dev/hid/hid.c
+++ sys/dev/hid/hid.c
@@ -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;
@@ -192,8 +192,7 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
{
struct hid_item *c;
unsigned int bTag, bType, bSize;
- uint32_t oldpos;
- int32_t mask;
+ uint32_t oldpos, uval;
int32_t dval;
if (s == NULL)
@@ -249,7 +248,6 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
/* get next item */
while (s->p != s->end) {
-
bSize = hid_get_byte(s, 1);
if (bSize == 0xfe) {
/* long item */
@@ -267,34 +265,34 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
}
switch (bSize) {
case 0:
- dval = 0;
- mask = 0;
+ uval = 0;
+ dval = uval;
break;
case 1:
- dval = hid_get_byte(s, 1);
- mask = 0xFF;
+ uval = hid_get_byte(s, 1);
+ dval = (int8_t)uval;
break;
case 2:
- dval = hid_get_byte(s, 1);
- dval |= hid_get_byte(s, 1) << 8;
- mask = 0xFFFF;
+ uval = hid_get_byte(s, 1);
+ uval |= hid_get_byte(s, 1) << 8;
+ dval = (int16_t)uval;
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;
- mask = 0xFFFFFFFF;
+ 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 = (int32_t)uval;
break;
default:
- dval = hid_get_byte(s, bSize);
+ uval = hid_get_byte(s, bSize);
DPRINTF("bad length %u (data=0x%02x)\n",
- bSize, dval);
+ bSize, uval);
continue;
}
- DPRINTF("%s: bType=%d bTag=%d dval=%d\n", __func__,
- bType, bTag, dval);
+ DPRINTF("%s: bType=%d bTag=%d dval=%d uval=%u\n", __func__,
+ bType, bTag, dval, uval);
switch (bType) {
case 0: /* Main */
switch (bTag) {
@@ -330,7 +328,7 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
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 +354,7 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
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 +369,19 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
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;
break;
case 8:
- hid_switch_rid(s, c, dval & mask);
+ hid_switch_rid(s, c, dval);
break;
case 9:
- /* mask because value is unsigned */
- s->loc_count = dval & mask;
+ s->loc_count = uval;
break;
case 10: /* Push */
if (s->pushlevel < MAXPUSH - 1) {
@@ -428,14 +424,14 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
switch (bTag) {
case 0:
if (bSize != 4)
- dval = (dval & mask) | c->_usage_page;
+ uval = c->_usage_page | uval;
/* 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 +444,16 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
s->susage |= 1;
if (bSize != 4)
- dval = (dval & mask) | c->_usage_page;
- c->usage_minimum = dval;
+ uval = c->_usage_page | uval;
+ 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 = c->_usage_page | uval;
+ c->usage_maximum = uval;
check_set:
if (s->susage != 3)
@@ -478,25 +474,25 @@ hid_get_item(struct hid_data *s, struct hid_item *h)
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);
@@ -545,7 +541,7 @@ hid_report_size(const void *buf, int len, enum hid_kind k, u_int8_t id)
}
int
-hid_locate(const void *desc, int size, int32_t u, uint8_t id, enum hid_kind k,
+hid_locate(const void *desc, int size, uint32_t u, uint8_t id, enum hid_kind k,
struct hid_location *loc, uint32_t *flags)
{
struct hid_data *d;
@@ -683,7 +679,7 @@ hid_get_collection_data(const void *desc, int size, int32_t usage,
}
int
-hid_get_id_of_collection(const void *desc, int size, int32_t usage,
+hid_get_id_of_collection(const void *desc, int size, uint32_t usage,
uint32_t collection)
{
struct hid_data *hd;
diff --git sys/dev/hid/hid.h sys/dev/hid/hid.h
index 7f4d38f6f74..014c7526734 100644
--- sys/dev/hid/hid.h
+++ sys/dev/hid/hid.h
@@ -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;
@@ -88,13 +88,13 @@ struct hid_data *hid_start_parse(const void *, int, enum hid_kind);
void hid_end_parse(struct hid_data *);
int hid_get_item(struct hid_data *, struct hid_item *);
int hid_report_size(const void *, int, enum hid_kind, uint8_t);
-int hid_locate(const void *, int, int32_t, uint8_t, enum hid_kind,
+int hid_locate(const void *, int, uint32_t, uint8_t, enum hid_kind,
struct hid_location *, uint32_t *);
int32_t hid_get_data(const uint8_t *buf, int, struct hid_location *);
uint32_t hid_get_udata(const uint8_t *buf, int, struct hid_location *);
int hid_is_collection(const void *, int, uint8_t, int32_t);
struct hid_data *hid_get_collection_data(const void *, int, int32_t, uint32_t);
-int hid_get_id_of_collection(const void *, int, int32_t, uint32_t);
+int hid_get_id_of_collection(const void *, int, uint32_t, uint32_t);
int hid_find_report(const void *, int len, enum hid_kind, int32_t,
int, int32_t *, int32_t *);
diff --git sys/dev/hid/hidmtvar.h sys/dev/hid/hidmtvar.h
index 1b799bff875..724bff0e1cb 100644
--- sys/dev/hid/hidmtvar.h
+++ sys/dev/hid/hidmtvar.h
@@ -16,7 +16,7 @@
*/
struct hidmt_data {
- int32_t usage;
+ uint32_t usage;
struct hid_location loc;
SIMPLEQ_ENTRY(hidmt_data) entry;
};
diff --git sys/dev/i2c/ihidev.c sys/dev/i2c/ihidev.c
index c922b78573b..ac07767fad5 100644
--- sys/dev/i2c/ihidev.c
+++ sys/dev/i2c/ihidev.c
@@ -799,7 +799,7 @@ ihidev_maxrepid(void *buf, int len)
maxid = -1;
h.report_ID = 0;
for (d = hid_start_parse(buf, len, hid_all); hid_get_item(d, &h);)
- if (h.report_ID > maxid)
+ if ((int)h.report_ID > maxid)
maxid = h.report_ID;
hid_end_parse(d);
diff --git sys/dev/usb/uhidev.c sys/dev/usb/uhidev.c
index 37f60cb8b53..83b681a12bf 100644
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -402,7 +402,7 @@ uhidev_maxrepid(void *buf, int len)
maxid = -1;
h.report_ID = 0;
for (d = hid_start_parse(buf, len, hid_all); hid_get_item(d, &h);)
- if (h.report_ID > maxid)
+ if ((int)h.report_ID > maxid)
maxid = h.report_ID;
hid_end_parse(d);
return (maxid);
signedness fixes in hid(4)