From mboxrd@z Thu Jan 1 00:00:00 1970 From: b_lkasam@codeaurora.org Subject: Re: ALSA core info race condition Date: Wed, 28 Jun 2017 11:28:07 +0530 Message-ID: References: <70ff31e5f7fb35774cc0a9f3764ed144@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by alsa0.perex.cz (Postfix) with ESMTP id 64981266838 for ; Wed, 28 Jun 2017 07:58:09 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: alsa-devel@alsa-project.org, lkasam@qti.qualcomm.com List-Id: alsa-devel@alsa-project.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