[bug report] ALSA: usb-audio: More validations of descriptor units

[bug report] ALSA: usb-audio: More validations of descriptor units

[Dan Carpenter]

Hello Takashi Iwai,

This is a semi-automatic email about new static checker warnings.

The patch 57f8770620e9: "ALSA: usb-audio: More validations of
descriptor units" from Aug 20, 2019, leads to the following Smatch
complaint:

    sound/usb/quirks.c:254 create_yamaha_midi_quirk()
    warn: variable dereferenced before check 'injd' (see line 251)

sound/usb/quirks.c
   244          /* must have some valid jack descriptors */
   245          injd = snd_usb_find_csint_desc(alts->extra, alts->extralen,
   246                                         NULL, USB_MS_MIDI_IN_JACK);
   247          outjd = snd_usb_find_csint_desc(alts->extra, alts->extralen,
   248                                          NULL, USB_MS_MIDI_OUT_JACK);
   249          if (!injd && !outjd)
                     ^^^^^^^^^^^^^
At least one must be valid.

   250                  return -ENODEV;
   250			return -ENODEV;
   251		if (!snd_usb_validate_midi_desc(injd) ||
   252		    !snd_usb_validate_midi_desc(outjd))

So this should return true/valid if the pointer is NULL?

   253			return -ENODEV;
   254		if (injd && (injd->bLength < 5 ||
   255			     (injd->bJackType != USB_MS_EMBEDDED &&
   256			      injd->bJackType != USB_MS_EXTERNAL)))

regards,
dan carpenter

Re: [bug report] ALSA: usb-audio: More validations of descriptor units

[Takashi Iwai]

On Mon, 26 Aug 2019 13:48:46 +0200,
Dan Carpenter wrote:
> 
> Hello Takashi Iwai,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 57f8770620e9: "ALSA: usb-audio: More validations of
> descriptor units" from Aug 20, 2019, leads to the following Smatch
> complaint:
> 
>     sound/usb/quirks.c:254 create_yamaha_midi_quirk()
>     warn: variable dereferenced before check 'injd' (see line 251)
> 
> sound/usb/quirks.c
>    244          /* must have some valid jack descriptors */
>    245          injd = snd_usb_find_csint_desc(alts->extra, alts->extralen,
>    246                                         NULL, USB_MS_MIDI_IN_JACK);
>    247          outjd = snd_usb_find_csint_desc(alts->extra, alts->extralen,
>    248                                          NULL, USB_MS_MIDI_OUT_JACK);
>    249          if (!injd && !outjd)
>                      ^^^^^^^^^^^^^
> At least one must be valid.
> 
>    250                  return -ENODEV;
>    250			return -ENODEV;
>    251		if (!snd_usb_validate_midi_desc(injd) ||
>    252		    !snd_usb_validate_midi_desc(outjd))
> 
> So this should return true/valid if the pointer is NULL?
> 
>    253			return -ENODEV;
>    254		if (injd && (injd->bLength < 5 ||
>    255			     (injd->bJackType != USB_MS_EMBEDDED &&
>    256			      injd->bJackType != USB_MS_EXTERNAL)))
> 
> regards,
> dan carpenter

Thanks for the report.  The fix patch is below.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: usb-audio: Fix possible NULL dereference at
 create_yamaha_midi_quirk()

The previous addition of descriptor validation may lead to a NULL
dereference at create_yamaha_midi_quirk() when either injd or outjd is
NULL.  Add proper non-NULL checks.

Fixes: 57f8770620e9 ("ALSA: usb-audio: More validations of descriptor units")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/quirks.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 7e9735aa7ac9..5c0fa5cf1987 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -248,8 +248,8 @@ static int create_yamaha_midi_quirk(struct snd_usb_audio *chip,
 					NULL, USB_MS_MIDI_OUT_JACK);
 	if (!injd && !outjd)
 		return -ENODEV;
-	if (!snd_usb_validate_midi_desc(injd) ||
-	    !snd_usb_validate_midi_desc(outjd))
+	if (!(injd && snd_usb_validate_midi_desc(injd)) ||
+	    !(outjd && snd_usb_validate_midi_desc(outjd)))
 		return -ENODEV;
 	if (injd && (injd->bLength < 5 ||
 		     (injd->bJackType != USB_MS_EMBEDDED &&
-- 
2.16.4