Re: Oops in sound/usb/pcm.c:match_endpoint_audioformats() in current -git

[Takashi Iwai <tiwai@suse.de>]

At Thu, 10 Jan 2013 20:45:02 +0100 (CET),
Eldad Zack wrote:
> 
> 
> On Thu, 10 Jan 2013, Takashi Iwai wrote:
> 
> > At Thu, 10 Jan 2013 13:49:22 +0100,
> > Jens Axboe wrote:
> > > 
> > > Here it is, it's from the one introducing the audioformat lookup.
> > > Confirmed that 3.8-rc3 with this backed out works fine, too. So should
> > > be fairly confident in that result.
> 
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] ALSA: usb-audio: Fix NULL dereference by access to
> >  non-existing substream
> > 
> > The commit [0d9741c0: ALSA: usb-audio: sync ep init fix for
> > audioformat mismatch] introduced the correction of parameters to be
> > set for sync EP.  But since the new code assumes that the sync EP is
> > always paired with the data EP of another direction, it triggers Oops
> > when a device only with a single direction is used.
> 
> Yes - sorry, I didn't consider this at all.
> 
> > This patch adds a proper check of sync EP type and the presence of the
> > paired substream for avoiding the crash.
> > 
> > Reported-by: Jens Axboe <axboe@kernel.dk>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/usb/pcm.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> > index c659310..21c0001 100644
> > --- a/sound/usb/pcm.c
> > +++ b/sound/usb/pcm.c
> > @@ -511,6 +511,17 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs)
> >  	struct snd_usb_substream *sync_subs =
> >  		&subs->stream->substream[subs->direction ^ 1];
> >  
> > +	if (subs->sync_endpoint->type != SND_USB_ENDPOINT_TYPE_DATA ||
> > +	    !subs->stream) {
> > +		ret = snd_usb_endpoint_set_params(subs->sync_endpoint,
> > +						  subs->pcm_format,
> > +						  subs->channels,
> > +						  subs->period_bytes,
> > +						  subs->cur_rate,
> > +						  subs->cur_audiofmt,
> > +						  NULL);
> > +	}
> > +
> 
> I think you want to return here, no?

Ah, yes, good catch.  It was dropped during rebasing and rewriting.
Below is the revised patch.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v2] ALSA: usb-audio: Fix NULL dereference by access to
 non-existing substream

The commit [0d9741c0: ALSA: usb-audio: sync ep init fix for
audioformat mismatch] introduced the correction of parameters to be
set for sync EP.  But since the new code assumes that the sync EP is
always paired with the data EP of another direction, it triggers Oops
when a device only with a single direction is used.

This patch adds a proper check of sync EP type and the presence of the
paired substream for avoiding the crash.

Reported-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/pcm.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index c659310..d82e378 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -511,6 +511,16 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs)
 	struct snd_usb_substream *sync_subs =
 		&subs->stream->substream[subs->direction ^ 1];
 
+	if (subs->sync_endpoint->type != SND_USB_ENDPOINT_TYPE_DATA ||
+	    !subs->stream)
+		return snd_usb_endpoint_set_params(subs->sync_endpoint,
+						   subs->pcm_format,
+						   subs->channels,
+						   subs->period_bytes,
+						   subs->cur_rate,
+						   subs->cur_audiofmt,
+						   NULL);
+
 	/* Try to find the best matching audioformat. */
 	list_for_each_entry(fp, &sync_subs->fmt_list, list) {
 		int score = match_endpoint_audioformats(fp, subs->cur_audiofmt,
-- 
1.8.1