Re: general protection fault in snd_usb_create_mixer

[Dan Carpenter <dan.carpenter@oracle.com>]

On Thu, Nov 21, 2019 at 07:25:09AM -0800, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    46178223 usb: gadget: add raw-gadget interface
> git tree:       https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=176f9836e00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=99c88c44660624e7
> dashboard link: https://syzkaller.appspot.com/bug?extid=a36ab65c6653d7ccdd62
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1447d3bae00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17ef3a86e00000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a36ab65c6653d7ccdd62@syzkaller.appspotmail.com
> 
> usb 1-1: 208:241 : does not exist
> usb 1-1: 208:242 : does not exist
> usb 1-1: 208:243 : does not exist
> usb 1-1: 208:244 : does not exist
> usb 1-1: 208:245 : does not exist
> usb 1-1: 208:246 : does not exist
> usb 1-1: 208:247 : does not exist
> usb 1-1: 208:248 : does not exist
> usb 1-1: 208:249 : does not exist
> usb 1-1: 208:250 : does not exist

These error messages are because:

sound/usb/card.c
   128  static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int interface)
   129  {
   130          struct usb_device *dev = chip->dev;
   131          struct usb_host_interface *alts;
   132          struct usb_interface_descriptor *altsd;
   133          struct usb_interface *iface = usb_ifnum_to_if(dev, interface);
   134  
   135          if (!iface) {
   136                  dev_err(&dev->dev, "%u:%d : does not exist\n",
   137                          ctrlif, interface);
   138                  return -EINVAL;
   139          }

"iface" is NULL.  The caller ignores the -EINVAL (correctly I think).

Then the NULL dereference happens in snd_usb_mixer_controls_badd()

sound/usb/mixer.c
  2903  static int snd_usb_mixer_controls_badd(struct usb_mixer_interface *mixer,
  2904                                         int ctrlif)
  2905  {
  2906          struct usb_device *dev = mixer->chip->dev;
  2907          struct usb_interface_assoc_descriptor *assoc;
  2908          int badd_profile = mixer->chip->badd_profile;
  2909          struct uac3_badd_profile *f;
  2910          const struct usbmix_ctl_map *map;
  2911          int p_chmask = 0, c_chmask = 0, st_chmask = 0;
  2912          int i;
  2913  
  2914          assoc = usb_ifnum_to_if(dev, ctrlif)->intf_assoc;
  2915  
  2916          /* Detect BADD capture/playback channels from AS EP descriptors */
  2917          for (i = 0; i < assoc->bInterfaceCount; i++) {
  2918                  int intf = assoc->bFirstInterface + i;
  2919  
  2920                  struct usb_interface *iface;
  2921                  struct usb_host_interface *alts;
  2922                  struct usb_interface_descriptor *altsd;
  2923                  unsigned int maxpacksize;
  2924                  char dir_in;
  2925                  int chmask, num;
  2926  
  2927                  if (intf == ctrlif)
  2928                          continue;
  2929  
  2930                  iface = usb_ifnum_to_if(dev, intf);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
iface is not checked.

Should it be "if (!iface) continue;?"

  2931                  num = iface->num_altsetting;
  2932  
  2933                  if (num < 2)
  2934                          return -EINVAL;
  2935  

I'm not sure.

regards,
dan carpenter