Re: ALSA core info race condition

[b_lkasam@codeaurora.org]

On 2017-06-28 11:06, Takashi Iwai wrote:
> On Tue, 27 Jun 2017 21:12:18 +0200,
> b_lkasam@codeaurora.org wrote:
>> 
>> hi ALSA team,
>> there is a race condition in below API when accessing list API.
>> 
>> In file sound/core/info.c:
>> 
>> Added below patch to avoid list access of same parent node
>> by two threads at same time causing list_debug crash.
>> 
>> diff --git a/sound/core/info.c b/sound/core/info.c
>> index b5158b5..c1fd671 100644
>> --- a/sound/core/info.c
>> +++ b/sound/core/info.c
>> @@ -747,8 +747,11 @@ snd_info_create_entry(const char *name, struct
>> snd_info_entry *parent)
>>  	INIT_LIST_HEAD(&entry->children);
>>  	INIT_LIST_HEAD(&entry->list);
>>  	entry->parent = parent;
>> -	if (parent)
>> +	if (parent) {
>> +		mutex_lock(&parent->access);
>>  		list_add_tail(&entry->list, &parent->children);
>> +		mutex_unlock(&parent->access);
>> +	}
>>  	return entry;
>>  }
>> 
>> Please check above logic looks fine, and help comment accordingly.
> 
> Have you ever got the actual crash?
> The function is supposed to be called only at probing, and its link
> base is the card object, so it's never called concurrently or the
> concurrency should be managed in the caller side.
> 
> Your "fix" looks OK, but it's likely superfluous from the actual
> usage.  Still it might be safer to add a protection, though.
> 
> 
> thanks,
> 
> Takashi


Yes Takashi, we found this crash happened on Qualcomm platform.
And as mentioned by you...it is used at bootup probe but from two 
different contexts,
Even if I try to manage at client level ...it will be only workaround. 
so actual fix
should be the patch I shared to avoid any similar issues in future.

thanks
Laxminath