linux-kernel - Re: Oops in sound/usb/pcm.c:match_endpoint

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hsj696tgw.wl%tiwai@suse.de>
Date:	Thu, 10 Jan 2013 14:46:55 +0100
From:	Takashi Iwai <tiwai@...e.de>
To:	Jens Axboe <axboe@...nel.dk>
Cc:	perex@...ex.cz, eldad@...refinery.com, alsa-devel@...a-project.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Oops in sound/usb/pcm.c:match_endpoint_audioformats() in current -git

At Thu, 10 Jan 2013 13:49:22 +0100,
Jens Axboe wrote:
> 
> On 2013-01-10 11:21, Takashi Iwai wrote:
> > At Thu, 10 Jan 2013 09:23:48 +0100,
> > Jens Axboe wrote:
> >>
> >> Hi,
> >>
> >> I have a USB DAC that I use for music. Just upgraded my workstation from
> >> 3.7-rc7 to 3.8-rc3 this morning, and when the DAC is switched on, I get
> >> an immediate oops. I have attached the picture. It's crashing here:
> >>
> >> (gdb) l *snd_usb_pcm_prepare+0x153
> >> 0x161c is in snd_usb_pcm_prepare (sound/usb/pcm.c:470).
> >> 465		snd_pcm_format_t pcm_format)
> >> 466	{
> >> 467		int i;
> >> 468		int score = 0;
> >> 469	
> >> 470		if (fp->channels < 1) {
> >> 471			snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp);
> >> 472			return 0;
> >> 473		}
> >> 474	
> >>
> >> 'fp' is clearly NULL.
> > 
> > Hmm... it's a function called only from a single place in
> > configure_sync_endpoint() and it's in list_for_each_entry():
> > 
> > 	list_for_each_entry(fp, &sync_subs->fmt_list, list) {
> > 		int score = match_endpoint_audioformats(fp, subs->cur_audiofmt,
> > 			subs->cur_rate, subs->pcm_format);
> > 
> > so it must be hardly NULL in normal situations.
> > The only scenario I can imagine right now is that the whole structure
> > is still uninitialized while it's called.  If so, it's a race problem,
> > and shouldn't be new to 3.8.  A patch like below might paper over the
> > problem although it's far from perfect.
> > 
> > 
> >> Let me know if you want a bisect.
> > 
> > Yeah, that'll be really helpful.
> 
> 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.

Thanks!  I'm slowly understanding what's happening there.
Could you try the patch below?


Takashi

---
From: Takashi Iwai <tiwai@...e.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.

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@...nel.dk>
Signed-off-by: Takashi Iwai <tiwai@...e.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);
+	}
+
 	/* 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/