Re: Crashes in uaudio

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

Hi,

>                                      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).

Looking at the initialisation of the audio descriptors in uaudio_process_as(),
the descripters are pointers into a larger buffer, and we already check the
length against the buffer size, for example:

  http://nxr.netbsd.org/source/xref/src/sys/dev/usb/uaudio.c#1578

so the attached patch increases the space for frequencies at the end of
usb_audio_streaming_type1_descriptor (to AUFMT_MAX_FREQUENCIES), and sets
auf->frequency_type to AUFMT_MAX_FREQUENCIES if the hardware reports more
than we can handle.  Because of the change to the descriptor definition,
we need an extra include in umidi.c and umidi_quirks.c, but I think that
this is simpler than splitting out the definition of AUFMT_MAX_FREQUENCIES
into a separate uaudiovar.h.

Comments?

Thanks,

J

PS.  Patch is against the jmcneill-usbmp branch, but the bug is in current
too.

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

cvs diff: Diffing .
Index: uaudio.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.124.2.5
diff -u -r1.124.2.5 uaudio.c
--- uaudio.c    29 Apr 2012 23:05:01 -0000      1.124.2.5
+++ uaudio.c    14 May 2012 09:51:16 -0000
@@ -1847,12 +1847,8 @@
                                        aprint_error("%s: please increase "
                                               "AUFMT_MAX_FREQUENCIES to %d\n",
                                               __func__, t1desc->bSamFreqType);
-                                       break;
-                               }
-                               if (j >= 2) {
-                                       aprint_error("%s: too much tSamFreq: "
-                                              "%d\n",
-                                              __func__, t1desc->bSamFreqType);
+                                       auf->frequency_type =
+                                           AUFMT_MAX_FREQUENCIES;
                                        break;
                                }
                                auf->frequency[j] = UA_GETSAMP(t1desc, j);
Index: uaudioreg.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uaudioreg.h,v
retrieving revision 1.15
diff -u -r1.15 uaudioreg.h
--- uaudioreg.h 28 Apr 2008 20:23:59 -0000      1.15
+++ uaudioreg.h 14 May 2012 09:51:16 -0000
@@ -113,7 +113,7 @@
        uByte           bBitResolution;
        uByte           bSamFreqType;
 #define UA_SAMP_CONTNUOUS 0
-       uByte           tSamFreq[3*2]; /* room for low and high */
+       uByte           tSamFreq[3*AUFMT_MAX_FREQUENCIES];
 #define UA_GETSAMP(p, n) ((p)->tSamFreq[(n)*3+0] | ((p)->tSamFreq[(n)*3+1] << 
8) | ((p)->tSamFreq[(n)*3+2] << 16))
 #define UA_SAMP_LO(p) UA_GETSAMP(p, 0)
 #define UA_SAMP_HI(p) UA_GETSAMP(p, 1)
Index: umidi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/umidi.c,v
retrieving revision 1.53.2.5
diff -u -r1.53.2.5 umidi.c
--- umidi.c     25 Feb 2012 10:26:24 -0000      1.53.2.5
+++ umidi.c     14 May 2012 09:51:17 -0000
@@ -52,6 +52,7 @@
 #include <dev/usb/usbdi.h>
 #include <dev/usb/usbdi_util.h>
 
+#include <dev/auconv.h>
 #include <dev/usb/usbdevs.h>
 #include <dev/usb/uaudioreg.h>
 #include <dev/usb/umidireg.h>
Index: umidi_quirks.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/umidi_quirks.c,v
retrieving revision 1.16.32.1
diff -u -r1.16.32.1 umidi_quirks.c
--- umidi_quirks.c      18 Feb 2012 07:35:10 -0000      1.16.32.1
+++ umidi_quirks.c      14 May 2012 09:51:17 -0000
@@ -49,6 +49,7 @@
 #include <dev/usb/usbdi.h>
 #include <dev/usb/usbdi_util.h>
 
+#include <dev/auconv.h>
 #include <dev/usb/usbdevs.h>
 #include <dev/usb/uaudioreg.h>
 #include <dev/usb/umidireg.h>