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

From: Jens Axboe
Date: Thu Jan 10 2013 - 15:26:10 EST


On 2013-01-10 21:19, Takashi Iwai wrote:
> 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@xxxxxxx>
>>> 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@xxxxxxxxx>
>>> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
>>> ---
>>> 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, I'll give it a go and report back.

--
Jens Axboe

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