From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B606DC32793 for ; Wed, 18 Jan 2023 12:33:35 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id E1DDE3AE8; Wed, 18 Jan 2023 13:32:42 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz E1DDE3AE8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1674045212; bh=4RhdeClS6kAVcpIBRh+EsfZIIB/Fcn81bErrD0/Q3/s=; h=Date:From:To:Subject:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: Cc:From; b=GH8Pg7JghHji3oUPfU8bShJfiOxmHU14SD/IjZ93rqWLMOCNGk9NLEfimmkR6pSPG 9pwkSAsUCIaYhKezGOM28MYRx/VoViRzvdf4XgJbjoXYIaYcCzs37JsanoQlALNrSX F4I9raOgyhaS+cL3x6lOQgWTSrvxZKEH3SAvPUh0= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 86C07F8024D; Wed, 18 Jan 2023 13:32:42 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 6105CF8026D; Wed, 18 Jan 2023 13:32:40 +0100 (CET) Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id DC140F8024C for ; Wed, 18 Jan 2023 13:32:34 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz DC140F8024C Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key, unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=KcfJjIS2; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=7GYrhje2 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 9ECDE3F3F6; Wed, 18 Jan 2023 12:32:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1674045153; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=UQ6JkWuiztCAnTB1i2sZeJAKraiEbhDdeBjF/yWAKuA=; b=KcfJjIS2H1m0lOrzFWkglxLlY1w9UH1pTnZxau5XPkrUgThqBKLmQjD0e6b9toUAMAJEOY 6O6X/fsGlt4RcS+01A2wQYmBkz8c28FO60QIjo3CgHrSvFRCMjGSqlpeQGuRj2unQfZjmN 0lcwudOvmbkVGaxTOEgrAt48S+rY0MQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1674045153; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=UQ6JkWuiztCAnTB1i2sZeJAKraiEbhDdeBjF/yWAKuA=; b=7GYrhje2dPXtBT+SaZWVGEu8JnheADGCy9OLqAEwI+kuCXguhNn2XyUBzoVe+Tk+96LT+t qW+ZcqXbzOtAdBDA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 85971138FE; Wed, 18 Jan 2023 12:32:33 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id VRAjIOHmx2O8HwAAMHmgww (envelope-from ); Wed, 18 Jan 2023 12:32:33 +0000 Date: Wed, 18 Jan 2023 13:32:33 +0100 Message-ID: <87h6wot44e.wl-tiwai@suse.de> From: Takashi Iwai To: Cezary Rojewski Subject: Re: [PATCH] ALSA: hda: Do not unset preset when cleaning up codec In-Reply-To: <22911d1c-02d0-e70d-63eb-44328d131c30@intel.com> References: <20230117154734.950487-1-cezary.rojewski@intel.com> <87edrt6tg3.wl-tiwai@suse.de> <22911d1c-02d0-e70d-63eb-44328d131c30@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, pierre-louis.bossart@linux.intel.com, tiwai@suse.com, hdegoede@redhat.com, broonie@kernel.org, amadeuszx.slawinski@linux.intel.com Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Wed, 18 Jan 2023 13:04:05 +0100, Cezary Rojewski wrote: > > On 2023-01-17 5:01 PM, Takashi Iwai wrote: > > ... > > >> This is a continuation of a discussion that begun in the middle of 2022 > >> [1] and was part of a larger series addressing several HDAudio topics. > >> > >> Single rmmod on ASoC's codec driver module is enough to cause a panic. > >> Given our results, no regression shows up with modprobe/rmmod on > >> snd_hda_intel side with this patch applied. > > > > I think one possible regression by this change would be the case you > > reload another codec driver. With keeping codec->preset, it's still > > thought as if already matched, and a wrong one could be used. > > > > And, this would be nothing but a leak of the possibly freed address. > > After hda_codec_driver_remove(), card->preset may point to an already > > freed address. > > > > So, just removing isn't right. It has to be cleared somewhere > > instead, e.g. in hda_bind.c. > > > > But, one thing I'm still concerned is that your comment about the call > > without the card binding. Do you mean that the > > snd_hda_codec_cleanup_for_unbind() may be called even if codec->card > > isn't set? > > One proposition would be to add line: > codec->preset = NULL; > > after both invocations of snd_hda_codec_cleanup_for_unbind() in > hda_codec_driver_probe/remove() in sound/pci/hda/hda_bind.c. Yes, that's what I meant, too. > In regard to your last question - no, cleanup is only called either in > component->probe()'s error path or in component->remove() once > snd_hda_codec_device_new() succeeds and thus, codec->card points to a > valid sound card. > > What I meant by my comment, is that removal of any components of an > ASoC sound card will cause all other components to have their > ->remove() op called too. Let's say I unload snd_soc_avs_hdaudio > module without unloading snd_soc_avs. As bus (snd_soc_avs) owns the > codecs, its platform component->remove() gets called right after codec > component->remove() but the actual devices are still present in the > system even with the sound card module (snd_soc_avs_hdaudio) unloaded. OK, then the snd_hda_codec object itself is kept bound on the bus, hence the call of snd_hda_codec_cleanup_for_unbind() is somehow inappropriate. The function could be renamed for avoid misunderstanding. thanks, Takashi