From 22f0cad4f589288874aeb435a885a1449392b4e1 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 08:20:04 -0700 Subject: [PATCH 1/9] ASoC: topology: Use the correct dobj to free enum control values and texts "dobj" is the widget dobj. dvalues and dtexts belong to the enum kcontrol's dobj (se->dobj). So, use this to free the control values and texts to prevent the NULL pointer dereference while removing widgets. Signed-off-by: Ranjani Sridharan --- sound/soc/soc-topology.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 472f7705da9368..0f6a6ad01570c3 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -482,10 +482,11 @@ static void remove_widget(struct snd_soc_component *comp, snd_ctl_remove(card, kcontrol); - kfree(dobj->control.dvalues); + /* free enum kcontrol's dvalues and dtexts */ + kfree(se->dobj.control.dvalues); for (j = 0; j < se->items; j++) - kfree(dobj->control.dtexts[j]); - kfree(dobj->control.dtexts); + kfree(se->dobj.control.dtexts[j]); + kfree(se->dobj.control.dtexts); kfree(se); kfree(w->kcontrol_news[i].name); From 05fb82de61ea777a4df86714ccfc933447d3d9e0 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 08:28:53 -0700 Subject: [PATCH 2/9] ASoC: core: do not unload topology in unregister_component() Topology is loaded when the component driver is probed. So, it should be removed in the component driver's remove(). This will prevent null pointer dereference while unregistering the component driver. Signed-off-by: Ranjani Sridharan --- sound/soc/soc-core.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 6f4842977b8d0d..ffd4db97a2ee90 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3327,8 +3327,6 @@ static int __snd_soc_unregister_component(struct device *dev) if (dev != component->dev) continue; - snd_soc_tplg_component_remove(component, - SND_SOC_TPLG_INDEX_ALL); snd_soc_component_del_unlocked(component); found = 1; break; From f6d96703dc2763d45bea827b54aa23896ec07983 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 08:29:49 -0700 Subject: [PATCH 3/9] ASoC: intel: skylake: add remove() callback for component driver Topology is not unloaded in the core during unregister_component() anymore. So, add the remove() callback that will unload the topology. Signed-off-by: Ranjani Sridharan --- sound/soc/intel/skylake/skl-pcm.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 56099db8f86ddf..a57d63373c7e8d 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -1462,9 +1462,17 @@ static int skl_platform_soc_probe(struct snd_soc_component *component) return 0; } +static void skl_pcm_remove(struct snd_soc_component *component) +{ + /* remove topology */ + snd_soc_tplg_component_remove(component, SND_SOC_TPLG_INDEX_ALL); +} + + static const struct snd_soc_component_driver skl_component = { .name = "pcm", .probe = skl_platform_soc_probe, + .remove = skl_pcm_remove, .ops = &skl_platform_ops, .pcm_new = skl_pcm_new, .pcm_free = skl_pcm_free, From 745a0076f6cdc73411806f4112b2495cd26b4aec Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 08:31:23 -0700 Subject: [PATCH 4/9] ASoC: core: remove link components before cleaning up card resources snd_soc_bind_card() calls snd_soc_instantiate_card() to first register the snd_card and then probe the link components by calling soc_probe_link_components(). The snd_card is used by the component driver during its probe() while loading topology widgets to add the controls to the snd_card. The snd_card also needs to be valid while unloading the topology widgets when the component driver is removed. Therefore, in snd_soc_unbind_card(), the link components should be removed with the call to soc_remove_link_components() before the card resources are cleaned up and the snd_card itself is freed. Signed-off-by: Ranjani Sridharan --- sound/soc/soc-core.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index ffd4db97a2ee90..04b5f49ec6bb06 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2831,10 +2831,22 @@ EXPORT_SYMBOL_GPL(snd_soc_register_card); static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) { + struct snd_soc_pcm_runtime *rtd; + int order; + if (card->instantiated) { card->instantiated = false; snd_soc_dapm_shutdown(card); snd_soc_flush_all_delayed_work(card); + + /* remove all components used by DAI links on this card */ + for_each_comp_order(order) { + for_each_card_rtds(card, rtd) { + soc_remove_link_components(card, rtd, order); + } + } + + /* clean up card resources */ soc_cleanup_card_resources(card); if (!unregister) list_add(&card->list, &unbind_card_list); From e5b1280724a11576c131ebb314b2208d22d60363 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 08:33:42 -0700 Subject: [PATCH 5/9] ASoC: SOF: remove topology in component driver remove() This is no longer done in the core during unregister_component. So, it should be done in the component driver's remove callback(). Signed-off-by: Ranjani Sridharan --- sound/soc/sof/pcm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index a083846139168b..e88bd3bbeeab1f 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -703,6 +703,8 @@ static int sof_pcm_probe(struct snd_soc_component *component) static void sof_pcm_remove(struct snd_soc_component *component) { + /* remove topology */ + snd_soc_tplg_component_remove(component, SND_SOC_TPLG_INDEX_ALL); } void snd_sof_new_platform_drv(struct snd_sof_dev *sdev) From 8e8ab0c182a4986da40fa8c09aa6e4bf6027a192 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 08:35:08 -0700 Subject: [PATCH 6/9] ASoC: SOF: core: make component driver resource-managed Previously, we couldn't make the component driver resource-managed because topology unloading was done by the core when the component driver was unregistered. But this has been fixed to allow topology removal during the component driver remove() callback and there aren't any NULL pointer dereferences anymore. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 2ca716f5dd8970..a46cfb9d18feef 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -350,7 +350,7 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) sdev->first_boot = false; /* now register audio DSP platform driver and dai */ - ret = snd_soc_register_component(sdev->dev, &sdev->plat_drv, + ret = devm_snd_soc_register_component(sdev->dev, &sdev->plat_drv, sof_ops(sdev)->drv, sof_ops(sdev)->num_drv); if (ret < 0) { @@ -461,16 +461,16 @@ int snd_sof_device_remove(struct device *dev) if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)) cancel_work_sync(&sdev->probe_work); - snd_soc_unregister_component(dev); snd_sof_fw_unload(sdev); snd_sof_ipc_free(sdev); snd_sof_free_debug(sdev); snd_sof_free_trace(sdev); snd_sof_remove(sdev); + /* - * platform_device_unregister() frees the card and its resources. - * So it should be called after unregistering the comp driver - * so that the card is valid while unregistering comp driver. + * Unregister machine driver. This will unbind the snd_card which + * will remove the component driver and unload the topology + * before freeing the snd_card. */ if (!IS_ERR_OR_NULL(pdata->pdev_mach)) platform_device_unregister(pdata->pdev_mach); From feae2d454a3c84072ca5e7d55c38cc63282b44d9 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 14:04:47 -0700 Subject: [PATCH 7/9] ASoC: core: rename ignore_module_refcount in component driver Recently, for Intel platforms the "ignore_module_refcount" field was introduced for the component driver which prevented the refcounts being incremented for the device driver module during component probe. However, this leads to the problem of allowing the device driver module to be unloaded while a pcm stream is open. So rename the field to "module_get_upon_open" which is more meaningful in terms of allowing the refcount for the module to be incremented when a pcm is opened. Also, align with the skylake component driver with the new name. Fixes: b450b878('ASoC: core: don't increase component module refcount unconditionally' Signed-off-by: Ranjani Sridharan --- include/sound/soc.h | 9 +++++++-- sound/soc/intel/skylake/skl-pcm.c | 2 +- sound/soc/soc-core.c | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index 1e2be35ed36f2b..482b4ea87c3c4b 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -802,8 +802,13 @@ struct snd_soc_component_driver { int probe_order; int remove_order; - /* signal if the module handling the component cannot be removed */ - unsigned int ignore_module_refcount:1; + /* + * signal if the module handling the component should not be removed + * if a pcm is open. Setting this would prevent the module + * refcount being incremented in probe() but allow it be incremented + * when a pcm is opened and decremented when it is closed. + */ + unsigned int module_get_upon_open:1; /* bits */ unsigned int idle_bias_on:1; diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index a57d63373c7e8d..ae44e378bda202 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -1476,7 +1476,7 @@ static const struct snd_soc_component_driver skl_component = { .ops = &skl_platform_ops, .pcm_new = skl_pcm_new, .pcm_free = skl_pcm_free, - .ignore_module_refcount = 1, /* do not increase the refcount in core */ + .module_get_upon_open = 1, /* increase the refcount when a pcm is opened */ }; int skl_platform_register(struct device *dev) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 04b5f49ec6bb06..79459527e5541a 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -947,7 +947,7 @@ static void soc_cleanup_component(struct snd_soc_component *component) snd_soc_dapm_free(snd_soc_component_get_dapm(component)); soc_cleanup_component_debugfs(component); component->card = NULL; - if (!component->driver->ignore_module_refcount) + if (!component->driver->module_get_upon_open) module_put(component->dev->driver->owner); } @@ -1381,7 +1381,7 @@ static int soc_probe_component(struct snd_soc_card *card, return 0; } - if (!component->driver->ignore_module_refcount && + if (!component->driver->module_get_upon_open && !try_module_get(component->dev->driver->owner)) return -ENODEV; From fe57de4d33559aabc1a8ff5e6ebf804c260640fe Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 14:08:54 -0700 Subject: [PATCH 8/9] ASoC: SOF: change the ignore_module_refcount to align with the new name The "ignore_module_refcount" field in the component driver has been renamed to "module_get_upon_open". So align with the new name. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/pcm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index e88bd3bbeeab1f..e54826c72726d6 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -729,6 +729,6 @@ void snd_sof_new_platform_drv(struct snd_sof_dev *sdev) pd->use_dai_pcm_id = true; pd->topology_name_prefix = "sof"; - /* do not increase the refcount in core */ - pd->ignore_module_refcount = 1; + /* increase the refcount when a pcm is opened */ + pd->module_get_upon_open = 1; } From 53c42c53687b03b5f2e64dc17fe30ad293b7f471 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Mon, 1 Apr 2019 14:11:57 -0700 Subject: [PATCH 9/9] ASoC: pcm: update module refcount if module_get_upon_open is set Setting the module_get_upon_open field for component driver prevents the module refcount from being incremented during component probe(). But this could lead to the module being allowed to be unloaded when a pcm stream is open. So, if this field is set, the module's refcount should be incremented/decremented during pcm open/close to prevent it. Signed-off-by: Ranjani Sridharan --- sound/soc/soc-pcm.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 7fe5321000e877..d84a9a5b14c000 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -463,6 +464,9 @@ static int soc_pcm_components_close(struct snd_pcm_substream *substream, continue; component->driver->ops->close(substream); + + if (component->driver->module_get_upon_open) + module_put(component->dev->driver->owner); } return 0; @@ -513,6 +517,10 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) !component->driver->ops->open) continue; + if (component->driver->module_get_upon_open) + if (!try_module_get(component->dev->driver->owner)) + return -ENODEV; + ret = component->driver->ops->open(substream); if (ret < 0) { dev_err(component->dev,