From 5a81f275cf141b70235000a888df15bdb42f9961 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 21 May 2021 17:29:21 -0400 Subject: [PATCH 1/2] Don't change number of HID devices until it's vetted --- shared-module/usb_hid/__init__.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index a92fd0c725..4367f938a0 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -160,11 +160,13 @@ bool common_hal_usb_hid_enable(const mp_obj_t devices) { return false; } - hid_devices_num = MP_OBJ_SMALL_INT_VALUE(mp_obj_len(devices)); - if (hid_devices_num > MAX_HID_DEVICES) { + const mp_int_t num_devices = MP_OBJ_SMALL_INT_VALUE(mp_obj_len(devices)); + if (num_devices > MAX_HID_DEVICES) { mp_raise_ValueError_varg(translate("No more than %d HID devices allowed"), MAX_HID_DEVICES); } + hid_devices_num = num_devices; + // Remember the devices in static storage so they live across VMs. for (mp_int_t i = 0; i < hid_devices_num; i++) { // devices has already been validated to contain only usb_hid_device_obj_t objects. From c6eb0283dc52e18876b04187fc3e37b8bc03aee2 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 21 May 2021 17:34:30 -0400 Subject: [PATCH 2/2] Don't set num_hid_devices before it's validated --- shared-module/usb_hid/__init__.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index 4367f938a0..18b9eeb8ee 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -79,7 +79,7 @@ static const uint8_t usb_hid_descriptor_template[] = { static supervisor_allocation *hid_report_descriptor_allocation; static usb_hid_device_obj_t hid_devices[MAX_HID_DEVICES]; // If 0, USB HID is disabled. -static mp_int_t hid_devices_num; +static mp_int_t num_hid_devices; // This tuple is store in usb_hid.devices. static mp_obj_tuple_t *hid_devices_tuple; @@ -97,7 +97,7 @@ static mp_obj_tuple_t default_hid_devices_tuple = { }; bool usb_hid_enabled(void) { - return hid_devices_num > 0; + return num_hid_devices > 0; } void usb_hid_set_defaults(void) { @@ -140,13 +140,13 @@ size_t usb_hid_add_descriptor(uint8_t *descriptor_buf, descriptor_counts_t *desc // Make up a fresh tuple containing the device objects saved in the static // devices table. Save the tuple in usb_hid.devices. static void usb_hid_set_devices_from_hid_devices(void) { - mp_obj_t tuple_items[hid_devices_num]; - for (mp_int_t i = 0; i < hid_devices_num; i++) { + mp_obj_t tuple_items[num_hid_devices]; + for (mp_int_t i = 0; i < num_hid_devices; i++) { tuple_items[i] = &hid_devices[i]; } // Remember tuple for gc purposes. - hid_devices_tuple = mp_obj_new_tuple(hid_devices_num, tuple_items); + hid_devices_tuple = mp_obj_new_tuple(num_hid_devices, tuple_items); usb_hid_set_devices(hid_devices_tuple); } @@ -165,10 +165,10 @@ bool common_hal_usb_hid_enable(const mp_obj_t devices) { mp_raise_ValueError_varg(translate("No more than %d HID devices allowed"), MAX_HID_DEVICES); } - hid_devices_num = num_devices; + num_hid_devices = num_devices; // Remember the devices in static storage so they live across VMs. - for (mp_int_t i = 0; i < hid_devices_num; i++) { + for (mp_int_t i = 0; i < num_hid_devices; i++) { // devices has already been validated to contain only usb_hid_device_obj_t objects. usb_hid_device_obj_t *device = MP_OBJ_TO_PTR(mp_obj_subscr(devices, MP_OBJ_NEW_SMALL_INT(i), MP_OBJ_SENTINEL)); @@ -185,7 +185,7 @@ void usb_hid_setup_devices(void) { usb_hid_set_devices_from_hid_devices(); // Create report buffers on the heap. - for (mp_int_t i = 0; i < hid_devices_num; i++) { + for (mp_int_t i = 0; i < num_hid_devices; i++) { usb_hid_device_create_report_buffers(&hid_devices[i]); } } @@ -193,12 +193,12 @@ void usb_hid_setup_devices(void) { // Total length of the report descriptor, with all configured devices. size_t usb_hid_report_descriptor_length(void) { size_t total_hid_report_descriptor_length = 0; - for (mp_int_t i = 0; i < hid_devices_num; i++) { + for (mp_int_t i = 0; i < num_hid_devices; i++) { total_hid_report_descriptor_length += hid_devices[i].report_descriptor_length; } // Don't need space for a report id if there's only one device. - if (hid_devices_num == 1) { + if (num_hid_devices == 1) { total_hid_report_descriptor_length -= 2; } return total_hid_report_descriptor_length; @@ -212,10 +212,10 @@ void usb_hid_build_report_descriptor(uint8_t *report_descriptor_space, size_t re uint8_t *report_descriptor_start = report_descriptor_space; - for (mp_int_t i = 0; i < hid_devices_num; i++) { + for (mp_int_t i = 0; i < num_hid_devices; i++) { usb_hid_device_obj_t *device = &hid_devices[i]; // Copy the report descriptor for this device. - if (hid_devices_num == 1) { + if (num_hid_devices == 1) { // There's only one device, so it shouldn't have a report ID. // Copy the descriptor, but splice out the report id indicator and value (2 bytes). memcpy(report_descriptor_start, device->report_descriptor, device->report_id_index - 1); @@ -260,13 +260,13 @@ void usb_hid_gc_collect(void) { gc_collect_ptr(hid_devices_tuple); // Mark any heap pointers in the static device list as in use. - for (mp_int_t i = 0; i < hid_devices_num; i++) { + for (mp_int_t i = 0; i < num_hid_devices; i++) { gc_collect_ptr(&hid_devices[i]); } } usb_hid_device_obj_t *usb_hid_get_device_with_report_id(uint8_t report_id) { - for (uint8_t i = 0; i < hid_devices_num; i++) { + for (uint8_t i = 0; i < num_hid_devices; i++) { usb_hid_device_obj_t *device = &hid_devices[i]; if (device->report_id == report_id) { return &hid_devices[i];