linux-kernel - Re: [PATCH] ASoC: core: Fix deferral of machine drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   09 Jan 2019 11:09:28 +0900
From:   Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To:     Jon Hunter <jonathanh@...dia.com>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>, <alsa-devel@...a-project.org>,
        <linux-kernel@...r.kernel.org>, <linux-tegra@...r.kernel.org>,
        Marcel Ziswiler <marcel@...wiler.com>,
        Matthias Reichl <hias@...us.com>
Subject: Re: [PATCH] ASoC: core: Fix deferral of machine drivers


Hi Jon

> Commit daecf46ee0e5 ("ASoC: soc-core: use snd_soc_dai_link_component for
> platform") added a new member to the snd_soc_dai_link structure for
> storing a pointer for a platform link component. The pointer for this
> platform link component was allocated, if not already populated by the
> machine driver, using devm_kzalloc() such that the memory would be
> automatically freed on error or removal of the soundcard. However, this
> introduced a new problem, because if the probing of the soundcard is
> deferred, then although the memory allocated for the platform link
> component is freed, if the snd_soc_dai_link structure is declared
> statically by the machine driver, then the pointer in the DAI link
> structure will never be clearer. This means that when the soundcard is
> probed again, memory for the platform link component will not be
> allocated again because the address of the pointer was not cleared
> and this causes sound core to access memory that is no longer valid.
> 
> In most cases this causes the following error condition to be triggered
> and causes probing the soundcard to fail.
> 
>  tegra-snd-sgtl5000 sound: ASoC: Both platform name/of_node are set for
>   sgtl5000
> 
> Unfortunately, because this platform link component is allocated before
> the DAI links are added to the soundcard, there is no easy way to clear
> this pointer on teardown if an error occurs.
> 
> The pointer for this platform link component was added for future
> proofing and communalising the structures for storing various data.
> Although a machine driver maybe used by more than one platform and so
> this platform data may vary from platform to platform, there is only
> ever a single instance for a given platform. Therefore, rather than
> dynamically allocate the platform link component structure, make it a
> static member of the snd_soc_dai_link to fix the problem.
> 
> It should be noted that if the platform_name of platform_of_node members
> of the snd_soc_dai_link structure are populated, these will always be
> used regardless of if the new platform.name or platform.of_node members
> are populated.
> 
> Fixes: daecf46ee0e5 ("ASoC: soc-core: use snd_soc_dai_link_component for platform")
> 
> Reported-by: Marcel Ziswiler <marcel.ziswiler@...adex.com>
> Signed-off-by: Jon Hunter <jonathanh@...dia.com>
> ---

Thank you for your patch.
The reason why it is allocating memory is it is for glue Legacy vs Modern style.
This means, this allocation style will be removed if ALSA SoC become modern style.
The missing part so far is CPU.
Only CPU is not yet supporting snd_soc_dai_link_component style.
If all CPU/Codec/Platform supports snd_soc_dai_link_component style,
all driver can switch to it, and then, all will be static style.
Currently, simple card series is only(?) using this style.

The reason why platform is using pointer style is that
someone (not me ;P) will support multi platform style in the future.

Best regards
---
Kuninori Morimoto