Re: general protection fault in snd_usb_create_mixer

[Takashi Iwai]

On Thu, 21 Nov 2019 21:49:56 +0100,
Dan Carpenter wrote:
> 
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > 
> > 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;?"

Right, that should fix the problem.  Although the interface number
referred in snd_usb_mixer_controls_badd() can be different from the
one checked in snd_usb_create_stream(), the culprit is the missing
NULL check there.


thanks,

Takashi