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-next>] [day] [month] [year] [list]

Date:	Thu, 10 Jan 2013 11:21:24 +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 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.

Or, at least, could you try to copy sound/usb/* from 3.7 to 3.8 and
see whether it's really a regression there?


thanks,

Takashi

---
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index ad181d5..3bfc564 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -81,8 +81,7 @@ static void snd_usb_audio_pcm_free(struct snd_pcm *pcm)
  */
 
 static void snd_usb_init_substream(struct snd_usb_stream *as,
-				   int stream,
-				   struct audioformat *fp)
+				   int stream)
 {
 	struct snd_usb_substream *subs = &as->substream[stream];
 
@@ -96,6 +95,13 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	subs->speed = snd_usb_get_speed(subs->dev);
 
 	snd_usb_set_pcm_ops(as->pcm, stream);
+}
+
+static void snd_usb_add_substream_format(struct snd_usb_stream *as,
+				   int stream,
+				   struct audioformat *fp)
+{
+	struct snd_usb_substream *subs = &as->substream[stream];
 
 	list_add_tail(&fp->list, &subs->fmt_list);
 	subs->formats |= fp->formats;
@@ -342,7 +348,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 		err = snd_pcm_new_stream(as->pcm, stream, 1);
 		if (err < 0)
 			return err;
-		snd_usb_init_substream(as, stream, fp);
+		snd_usb_add_substream_format(as, stream, fp);
 		return add_chmap(as->pcm, stream, subs);
 	}
 
@@ -370,7 +376,9 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	else
 		strcpy(pcm->name, "USB Audio");
 
-	snd_usb_init_substream(as, stream, fp);
+	snd_usb_init_substream(as, SNDRV_PCM_STREAM_PLAYBACK);
+	snd_usb_init_substream(as, SNDRV_PCM_STREAM_CAPTURE);
+	snd_usb_add_substream_format(as, stream, fp);
 
 	list_add(&as->list, &chip->pcm_list);
 	chip->pcm_devs++;
--
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/