Crashes in uaudio

[Julian Coleman <jdc%coris.org.uk@localhost>]

Hi,

I see kernel crashes when playing a sample via a USB audio device:

  uaudio0 at uhub0 port 2 configuration 1 interface 0
  uaudio0: Logitech Logitech USB Headset, rev 1.10/10.13, addr 3
  uaudio0: ignored input endpoint of type adaptive
  uaudio_identify_as: too much tSamFreq: 5
  uaudio_identify_as: too much tSamFreq: 5
  uaudio0: audio rev 1.00
  audio1 at uaudio0: full duplex, playback, capture, independent

note the "uaudio_identify_as: too much tSamFreq: 5" lines here.  With extra
debug printf(), I can see that the attach adds sample rates of 8000 and 11025
before the test added in r1.130 of uaudio.c [*] applies and breaks out of the
loop.  The problem is that the number of frequencies is still set to the old
value (i.e., frequency_type = 5) and when playing any audio, the frequency
list is looped over in auconv_rateconv_check_rates().  With extra debug there,
I see:

  auconv_rateconv_check_rates: sample_rate = 44100
  frequency[0] = 8000, minrate = 8000, maxrate = 8000
  frequency[1] = 8000, minrate = 8000, maxrate = 11025
  frequency[2] = 8000, minrate = 8000, maxrate = 778925428
  frequency[3] = 8000, minrate = 8000, maxrate = 1694498816
  frequency[4] = 8000, minrate = 8000, maxrate = 1694498816

this causes it to set a sample rate of 1694498816 (junk value).  This then
causes uaudio_chan_ptransfer() to try and add 67M bytes to the 64K channel
buffer.  The machine crashes when writing past the end of the channel buffer
into an unmapped page.

The simple solution here is to set frequency_type to 2 because we aborted
the loop in the test at line 1852 of src/sys/dev/usb/uaudio.c.  But, the
value 2 is significant here, because the gcc -O3 check notices that we
fall off the end of the tSamFreq[] array when calling UA_GETSAMP() (defined
in src/sys/dev/usb/uaudioreg.h) with a second parameter not 0 or 1.  But,
with this check commented out, I see that all 5 sample rates (8000, 11025,
22050, 44100, and 48000) are added.  So, it seems that we are overwriting
the end of usb_audio_streaming_type1_descriptor for every descriptor where
the number of rates is more than 2.  The problem is that we only notice when
we come to read.  So, I think that we should set:

        uByte           tSamFreq[3*AUFMT_MAX_FREQUENCIES];

in the usb_audio_streaming_type1_descriptor definition (uaudioreg.h), make
sure that we only copy the correct maximum number of bytes when setting it
up, and remove the test at line 1852 of uaudio.c.  We probably should also
set:

  auf->frequency_type = AUFMT_MAX_FREQUENCIES;

in the test at line 1846, just in case we do meet a device which advertises
more frequencies than we can handle (if we don't, we could end up reading
memory after the end of the usb_audio_streaming_type1_descriptor).

However, I'm not 100% sure that I've not missed anything else here.  Comments?

Thanks,

J

[*] 
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/usb/uaudio.c.diff?r1=1.129&r2=1.130

-- 
  My other computer also runs NetBSD    /        Sailing at Newbiggin
        http://www.netbsd.org/        /   http://www.newbigginsailingclub.org/