From 673a916cd0193ff869ca7385f02b699f771ceb06 Mon Sep 17 00:00:00 2001 From: rppicomidi Date: Sat, 5 Jul 2025 16:52:09 -0700 Subject: [PATCH 1/4] Fix #3159: Handle MIDI interface descriptor after audio streaming interface --- src/class/midi/midi_host.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/class/midi/midi_host.c b/src/class/midi/midi_host.c index cd6e115ee..77bacf24c 100644 --- a/src/class/midi/midi_host.c +++ b/src/class/midi/midi_host.c @@ -197,7 +197,6 @@ bool midih_xfer_cb(uint8_t dev_addr, uint8_t ep_addr, xfer_result_t result, uint //--------------------------------------------------------------------+ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *desc_itf, uint16_t max_len) { (void) rhport; - TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass); const uint8_t *p_end = ((const uint8_t *) desc_itf) + max_len; const uint8_t *p_desc = (const uint8_t *) desc_itf; @@ -211,7 +210,15 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d desc_cb.jack_num = 0; // There can be just a MIDI or an Audio + MIDI interface - // If there is Audio Control Interface + Audio Header descriptor, skip it + // If there is Audio Control Interface + Audio Header descriptor, then skip it. + // If there is an Audio Control Interface + Audio Streaming Interface, then + // ignore the Audio Streaming Interface. + // Future: + // Note that if this driver is used with an USB Audio Streaming host driver, + // then call that driver first. If the MIDI interface comes before the + // audio streaming interface, then the audio driver will have to call this + // driver after parsing the audio control interface and then resume parsing + // the streaming audio interface. if (AUDIO_SUBCLASS_CONTROL == desc_itf->bInterfaceSubClass) { TU_VERIFY(max_len > 2*sizeof(tusb_desc_interface_t) + sizeof(audio_desc_cs_ac_interface_t)); @@ -222,12 +229,21 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d p_desc = tu_desc_next(p_desc); desc_itf = (const tusb_desc_interface_t *)p_desc; - TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass); p_midi->itf_count = 1; - } + // See issue #3159 + while ((p_desc < p_end) && (tu_desc_next(p_desc) <= p_end) && (desc_itf->bDescriptorType != TUSB_DESC_INTERFACE || (desc_itf->bInterfaceClass == TUSB_CLASS_AUDIO && desc_itf->bInterfaceSubClass != AUDIO_SUBCLASS_MIDI_STREAMING))) + { + if (desc_itf->bDescriptorType == TUSB_DESC_INTERFACE && desc_itf->bAlternateSetting == 0) { + p_midi->itf_count++; + } + p_desc = tu_desc_next(p_desc); + desc_itf = (tusb_desc_interface_t const *)p_desc; + } + TU_VERIFY(p_desc < p_end); // If MIDI interface comes after Audio Streaming, then max_len did not include the MIDI interface descriptor + TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass); +} TU_VERIFY(AUDIO_SUBCLASS_MIDI_STREAMING == desc_itf->bInterfaceSubClass); - - TU_LOG_DRV("MIDI opening Interface %u (addr = %u)\r\n", desc_itf->bInterfaceNumber, dev_addr); + p_midi->bInterfaceNumber = desc_itf->bInterfaceNumber; p_midi->iInterface = desc_itf->iInterface; p_midi->itf_count++; @@ -236,6 +252,7 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d p_desc = tu_desc_next(p_desc); // next to CS Header bool found_new_interface = false; + printf("%p %p %p\r\n", p_desc, tu_desc_next(p_desc), p_end); while ((p_desc < p_end) && (tu_desc_next(p_desc) <= p_end) && !found_new_interface) { switch (tu_desc_type(p_desc)) { case TUSB_DESC_INTERFACE: From 0d080ca7bac9d3ed1c0f080c111e0530284e2ee9 Mon Sep 17 00:00:00 2001 From: rppicomidi Date: Tue, 22 Jul 2025 16:52:47 -0700 Subject: [PATCH 2/4] Delete debugging printf --- src/class/midi/midi_host.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/class/midi/midi_host.c b/src/class/midi/midi_host.c index 77bacf24c..1ac24eb05 100644 --- a/src/class/midi/midi_host.c +++ b/src/class/midi/midi_host.c @@ -252,7 +252,6 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d p_desc = tu_desc_next(p_desc); // next to CS Header bool found_new_interface = false; - printf("%p %p %p\r\n", p_desc, tu_desc_next(p_desc), p_end); while ((p_desc < p_end) && (tu_desc_next(p_desc) <= p_end) && !found_new_interface) { switch (tu_desc_type(p_desc)) { case TUSB_DESC_INTERFACE: From 9030fe43fae06b52a0d88584aaff156c8c0cb7b8 Mon Sep 17 00:00:00 2001 From: rppicomidi Date: Wed, 23 Jul 2025 06:46:24 -0700 Subject: [PATCH 3/4] Restore accidentally erased debug log message --- src/class/midi/midi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/class/midi/midi_host.c b/src/class/midi/midi_host.c index 1ac24eb05..2395a0ef8 100644 --- a/src/class/midi/midi_host.c +++ b/src/class/midi/midi_host.c @@ -243,7 +243,7 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass); } TU_VERIFY(AUDIO_SUBCLASS_MIDI_STREAMING == desc_itf->bInterfaceSubClass); - + TU_LOG_DRV("MIDI opening Interface %u (addr = %u)\r\n", desc_itf->bInterfaceNumber, dev_addr); p_midi->bInterfaceNumber = desc_itf->bInterfaceNumber; p_midi->iInterface = desc_itf->iInterface; p_midi->itf_count++; From 12a1d0e7ede97c793a5e11b3cce7284cdbe994c8 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 2 Aug 2025 11:23:15 +0700 Subject: [PATCH 4/4] use tu_desc_in_bounds() for descriptor loop --- src/class/midi/midi_host.c | 19 ++++++++------- src/class/vendor/vendor_device.c | 2 +- src/common/tusb_common.h | 38 ++++++++++++++++++++++++++++++ src/common/tusb_types.h | 40 -------------------------------- 4 files changed, 49 insertions(+), 50 deletions(-) diff --git a/src/class/midi/midi_host.c b/src/class/midi/midi_host.c index 2395a0ef8..50bda1e36 100644 --- a/src/class/midi/midi_host.c +++ b/src/class/midi/midi_host.c @@ -197,6 +197,7 @@ bool midih_xfer_cb(uint8_t dev_addr, uint8_t ep_addr, xfer_result_t result, uint //--------------------------------------------------------------------+ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *desc_itf, uint16_t max_len) { (void) rhport; + TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass); const uint8_t *p_end = ((const uint8_t *) desc_itf) + max_len; const uint8_t *p_desc = (const uint8_t *) desc_itf; @@ -210,9 +211,8 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d desc_cb.jack_num = 0; // There can be just a MIDI or an Audio + MIDI interface - // If there is Audio Control Interface + Audio Header descriptor, then skip it. - // If there is an Audio Control Interface + Audio Streaming Interface, then - // ignore the Audio Streaming Interface. + // - If there is Audio Control Interface + Audio Header descriptor, then skip it. + // - If there is an Audio Control Interface + Audio Streaming Interface, then ignore the Audio Streaming Interface. // Future: // Note that if this driver is used with an USB Audio Streaming host driver, // then call that driver first. If the MIDI interface comes before the @@ -230,19 +230,20 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d p_desc = tu_desc_next(p_desc); desc_itf = (const tusb_desc_interface_t *)p_desc; p_midi->itf_count = 1; - // See issue #3159 - while ((p_desc < p_end) && (tu_desc_next(p_desc) <= p_end) && (desc_itf->bDescriptorType != TUSB_DESC_INTERFACE || (desc_itf->bInterfaceClass == TUSB_CLASS_AUDIO && desc_itf->bInterfaceSubClass != AUDIO_SUBCLASS_MIDI_STREAMING))) - { + // skip non-interface and non-midi streaming descriptors + while (tu_desc_in_bounds(p_desc, p_end) && + (desc_itf->bDescriptorType != TUSB_DESC_INTERFACE || (desc_itf->bInterfaceClass == TUSB_CLASS_AUDIO && desc_itf->bInterfaceSubClass != AUDIO_SUBCLASS_MIDI_STREAMING))) { if (desc_itf->bDescriptorType == TUSB_DESC_INTERFACE && desc_itf->bAlternateSetting == 0) { p_midi->itf_count++; } p_desc = tu_desc_next(p_desc); desc_itf = (tusb_desc_interface_t const *)p_desc; } - TU_VERIFY(p_desc < p_end); // If MIDI interface comes after Audio Streaming, then max_len did not include the MIDI interface descriptor + TU_VERIFY(p_desc < p_end); // TODO: If MIDI interface comes after Audio Streaming, then max_len did not include the MIDI interface descriptor TU_VERIFY(TUSB_CLASS_AUDIO == desc_itf->bInterfaceClass); -} + } TU_VERIFY(AUDIO_SUBCLASS_MIDI_STREAMING == desc_itf->bInterfaceSubClass); + TU_LOG_DRV("MIDI opening Interface %u (addr = %u)\r\n", desc_itf->bInterfaceNumber, dev_addr); p_midi->bInterfaceNumber = desc_itf->bInterfaceNumber; p_midi->iInterface = desc_itf->iInterface; @@ -252,7 +253,7 @@ bool midih_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *d p_desc = tu_desc_next(p_desc); // next to CS Header bool found_new_interface = false; - while ((p_desc < p_end) && (tu_desc_next(p_desc) <= p_end) && !found_new_interface) { + while (tu_desc_in_bounds(p_desc, p_end) && !found_new_interface) { switch (tu_desc_type(p_desc)) { case TUSB_DESC_INTERFACE: found_new_interface = true; diff --git a/src/class/vendor/vendor_device.c b/src/class/vendor/vendor_device.c index 6f109c321..b0332b0fe 100644 --- a/src/class/vendor/vendor_device.c +++ b/src/class/vendor/vendor_device.c @@ -211,7 +211,7 @@ uint16_t vendord_open(uint8_t rhport, const tusb_desc_interface_t* desc_itf, uin TU_VERIFY(p_vendor, 0); p_vendor->itf_num = desc_itf->bInterfaceNumber; - while (tu_desc_is_valid(p_desc, desc_end)) { + while (tu_desc_in_bounds(p_desc, desc_end)) { const uint8_t desc_type = tu_desc_type(p_desc); if (desc_type == TUSB_DESC_INTERFACE || desc_type == TUSB_DESC_INTERFACE_ASSOCIATION) { break; // end of this interface diff --git a/src/common/tusb_common.h b/src/common/tusb_common.h index 0351a3d8f..489e09a15 100644 --- a/src/common/tusb_common.h +++ b/src/common/tusb_common.h @@ -324,6 +324,44 @@ TU_ATTR_ALWAYS_INLINE static inline void tu_unaligned_write16(void *mem, uint16_ + TU_BIN8(dlsb)) #endif +//--------------------------------------------------------------------+ +// Descriptor helper +//--------------------------------------------------------------------+ + +// return next descriptor +TU_ATTR_ALWAYS_INLINE static inline uint8_t const * tu_desc_next(void const* desc) { + uint8_t const* desc8 = (uint8_t const*) desc; + return desc8 + desc8[DESC_OFFSET_LEN]; +} + +// get descriptor length +TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_len(void const* desc) { + return ((uint8_t const*) desc)[DESC_OFFSET_LEN]; +} + +// get descriptor type +TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_type(void const* desc) { + return ((uint8_t const*) desc)[DESC_OFFSET_TYPE]; +} + +// get descriptor subtype +TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_subtype(void const* desc) { + return ((uint8_t const*) desc)[DESC_OFFSET_SUBTYPE]; +} + +TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_in_bounds(uint8_t const* p_desc, uint8_t const* desc_end) { + return (p_desc < desc_end) && (tu_desc_next(p_desc) <= desc_end); +} + +// find descriptor that match byte1 (type) +uint8_t const * tu_desc_find(uint8_t const* desc, uint8_t const* end, uint8_t byte1); + +// find descriptor that match byte1 (type) and byte2 +uint8_t const * tu_desc_find2(uint8_t const* desc, uint8_t const* end, uint8_t byte1, uint8_t byte2); + +// find descriptor that match byte1 (type) and byte2 +uint8_t const * tu_desc_find3(uint8_t const* desc, uint8_t const* end, uint8_t byte1, uint8_t byte2, uint8_t byte3); + #ifdef __cplusplus } #endif diff --git a/src/common/tusb_types.h b/src/common/tusb_types.h index ee97069bd..b3ef1e9c9 100644 --- a/src/common/tusb_types.h +++ b/src/common/tusb_types.h @@ -562,46 +562,6 @@ TU_ATTR_ALWAYS_INLINE static inline const char *tu_edpt_type_str(tusb_xfer_type_ } #endif -//--------------------------------------------------------------------+ -// Descriptor helper -//--------------------------------------------------------------------+ - -// return next descriptor -TU_ATTR_ALWAYS_INLINE static inline uint8_t const * tu_desc_next(void const* desc) { - uint8_t const* desc8 = (uint8_t const*) desc; - return desc8 + desc8[DESC_OFFSET_LEN]; -} - -// get descriptor length -TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_len(void const* desc) { - return ((uint8_t const*) desc)[DESC_OFFSET_LEN]; -} - -// get descriptor type -TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_type(void const* desc) { - return ((uint8_t const*) desc)[DESC_OFFSET_TYPE]; -} - -// get descriptor subtype -TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_subtype(void const* desc) { - return ((uint8_t const*) desc)[DESC_OFFSET_SUBTYPE]; -} - -TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_is_valid(void const* desc, uint8_t const* desc_end) { - const uint8_t* desc8 = (uint8_t const*) desc; - return (desc8 < desc_end) && (tu_desc_next(desc) <= desc_end); -} - - -// find descriptor that match byte1 (type) -uint8_t const * tu_desc_find(uint8_t const* desc, uint8_t const* end, uint8_t byte1); - -// find descriptor that match byte1 (type) and byte2 -uint8_t const * tu_desc_find2(uint8_t const* desc, uint8_t const* end, uint8_t byte1, uint8_t byte2); - -// find descriptor that match byte1 (type) and byte2 -uint8_t const * tu_desc_find3(uint8_t const* desc, uint8_t const* end, uint8_t byte1, uint8_t byte2, uint8_t byte3); - #ifdef __cplusplus } #endif