>Number:         32567
>Category:       kern
>Synopsis:       umidi(4) inconsistent treatment of cable number
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jan 19 00:45:01 +0000 2006
>Originator:     Chapman Flack
>Release:        2.0
>Organization:
>Environment:
NetBSD lundestad.anastigmatix.net 2.0 NetBSD 2.0 (lundestad) #86: Wed Jan 18 15:42:22 EST 2006  xxx@xxx:/usr/src/sys/arch/i386/compile/lundestad i386
>Description:
A USB MIDI device will support one or more USB 'endpoints' and an endpoint
can support up to sixteen 'jacks' each representing an independent MIDI
message stream. Messages for the jacks that share a common endpoint are
multiplexed by encapsulating them in packets that include a 4-bit Cable
Number (CN) identifying the jack, sending the packets on the common
endpoint, and demultiplexing them at the other end.

It should be clear from this discussion (16 jacks per endpoint, 4-bit
cable number) as well as the USB Device Class Definition for MIDI Devices,
section 4, that the CN is a per-endpoint identifier, and that a particular
jack is fully identified by <endpoint, CN on that endpoint>. However, the
code in umidi.c(alloc_all_jacks) currently assigns CNs in a global
sequence for the whole device, so that jacks on endpoints beyond the
first have cable numbers beginning above zero. On the other hand, in_intr
in the same file clearly assumes the CN is per endpoint, using it to
index a per-endpoint array of jacks, which could be an out of bounds index
in some cases given the way the CN is assigned.

Making matters murkier, there do seem to be nonstandard MIDI devices
that actually do expect a single global sequence of CNs--obviously
such devices will not have more than 16 jacks total--and so for those
devices the current behavior should be retained (but in_intr fixed
to avoid the indexing error). I don't have a wide variety of such
devices to test to see how prevalent that is, but because that is the
way the code has worked so far, to change it would probably break
support for some devices that now work.

So, I have introduced two complementary quirks, CN_SEQ_GLOBAL and
CN_SEQ_PER_EP. They are mutually exclusive and currently the default
if neither is given will be CN_SEQ_GLOBAL (the current behavior). The
point of adding two quirks for the same binary property is so either
behavior can be specified explicitly, which opens a path to later
changing the default to PER_EP without the need for a flag day.
in_intr is fixed to work correctly with either behavior (and will
report suspicious errors with a suggestion to check the CN SEQ quirk).
>How-To-Repeat:
I haven't actually seen a device that doesn't work on account of this
problem, but the inconsistency (and the fact that the array index in
in_intr would be out of bounds in some circumstances as a result) is
clear by inspection.
>Fix:
patch: http://www.anastigmatix.net/cvsweb.cgi/NetBSD/umidicn.pat

Patch is to 2.0 source after application of kern/32441 and 32442 patches.
CVS activity of the affected files has been very low so adapting to head
revisions should be easy (but 2.0 is what I've tested on).