[alsa-devel] [PATCH] ALSA: Fix double locking of card list in snd_card_register()

[Jaroslav Kysela]

On Thu, 4 Jun 2009, Jaroslav Kysela wrote:

> On Wed, 3 Jun 2009, Takashi Iwai wrote:
>
>> At Wed,  3 Jun 2009 20:43:29 +0100,
>> Mark Brown wrote:
>>>
>>> The introduction of snd_card_set_id() added a lock on the card list
>>> to the old choose_default_id() function when using it to implement
>>> the new API call. This lock is needed to allow us to walk the list
>>> and check to see if our new name is a duplicate. Unfortunately this
>>> causes a lockup when called from snd_card_register() (in cases
>>> where no ID is supplied for the card) since the card list is already
>>> locked there.
>>
>> Oh, thanks for catching this.  Obviously I haven't tested the
>> latest code enough...
>
> Yes, thanks.
>
>>> Fix this fairly hideously by factoring out the implementation and
>>> using a flag to indicate if the lock should be held. A better fix
>>> would probably be to refactor snd_card_register() to move the
>>> _set_id() outside the locking region but I can't immediately see
>>> anything I can convince myself is safe.
>>
>> Agreed, there can be a cleaner solution but your fix is definitely
>> safe.  So I applied it as a quick fix.
>
> The patch has a unlock problem in 'if (snd_cards[i] &&
> !strcmp(snd_cards[i]->id, id)) {' block.
>
> Anyway, here is next cleanup which will remove locking outside the
> internal id function and fixes also next possible race in unique id
> allocation:
>
> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=13d55b5cb27a3c35d0fb2a48e1ee10ec89488569

Oops. There was wrong "if (card->id[0] != '\0')" check so the 
call in snd_card_register() will not work as expected. This commit is 
final:

http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=c5313f60c4a50d6f0bb7d535310b654c4e9a449c

 						Jaroslav

-----
Jaroslav Kysela <perex at perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.