Messages in this thread | | | Subject | Re: [alsa-devel] [PATCH] ASoC: soc-core: Fix null pointer dereference in soc_find_component | From | Pierre-Louis Bossart <> | Date | Tue, 22 Jan 2019 14:11:45 -0600 |
| |
> The issue was that we were seeing a memory corruption bug on an AMD > chromebooks with that function already (not observed on Intel). I was > testing some SOF integrations and was seeing this in the kernel logs. > I had Dylan verify my logic before I sent the patch because it took so > long to identify the bug and it was traced to the patch that introduce > soc_init_platform. > > [ 10.922112] cz-da7219-max98357a AMD7219:00: ASoC: CPU DAI > designware-i2s.1.auto not registered > [ 10.922122] cz-da7219-max98357a AMD7219:00: > devm_snd_soc_register_card(acpd7219m98357) failed: -517 > [ 11.001411] cz-da7219-max98357a AMD7219:00: ASoC: Both platform > name/of_node are set for amd-max98357-play > [ 11.001423] cz-da7219-max98357a AMD7219:00: ASoC: failed to init > link amd-max98357-play > [ 11.001431] cz-da7219-max98357a AMD7219:00: > devm_snd_soc_register_card(acpd7219m98357) failed: -22 > [ 11.001577] cz-da7219-max98357a: probe of AMD7219:00 failed with error -22 > > of_node was never getting set but the pointer was becoming populated > (outside of the probe call) which traced to soc_init_platform function > which was not reallocating memory on a EPROBE_DEFER even though it was > getting freed by devm. I am not very familiar with devm but my local > maintainers say that it should be freeing the memory even on a > PROBE_DEFER. > The patch should mirror the memory behaviour in > snd_soc_init_multicodec which also reallocates its memory on every > probe. I'm not sure how the patch is causing you to defer, is your > component list corrupt? > > Sorry for the duplicate spam, forgot to send via plain text mode, > re-sending for the mailing list so it gets accepted.
There is no defer issue with the intel stuff, but we call this routine multiple times
snd_soc_register_card
--soc_init_dai_link
----snd_soc_init_platform
-- soc_soc_bind_card
----snd_soc_instantiate_card
------ soc_check_tplg_fes
-------- snd_soc_init_platform << ALLOC1
--------soc_init_dai_link
----------snd_soc_init_platform << ALLOC2
Initially dai_link->legacy_platform is 0, so gets set after the first first devm_kzalloc (ALLOC1) and after that we always allocate new memory (ALLOC2). The end result is that whatever we set in soc_check_tplg_fes is lost with the new/unnecessary alloc.
I would guess your solution is also a work-around, if devm_ effectively freed the memory then the pointer would become NULL. Or may that's the issue is that no one actually resets it.
| |