Skip to content

Commit

Permalink
ASoC: compress: Only call free for components which have been opened
Browse files Browse the repository at this point in the history
The core should only call free on a component if said component has
already had open called on it. This is not presently the case and most
compressed drivers in the kernel assume it will be. This causes null
pointer dereferences in the drivers as they attempt clean up for stuff
that was never put in place.

This is fixed by aborting calling open callbacks once a failure is
encountered and then during clean up only iterating through the
component list to that point.

This is a fairly quick fix to the issue, to allow backporting. There
is more refactoring to follow to tidy the code up a little.

Fixes: 9e7e373 ("ASoC: snd_soc_component_driver has snd_compr_ops")
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
  • Loading branch information
charleskeepax authored and broonie committed Apr 26, 2018
1 parent 483abac commit 572e6c8
Showing 1 changed file with 28 additions and 24 deletions.
52 changes: 28 additions & 24 deletions sound/soc/soc-compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
int ret = 0, __ret;
int ret;

mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);

Expand Down Expand Up @@ -68,16 +68,15 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
!component->driver->compr_ops->open)
continue;

__ret = component->driver->compr_ops->open(cstream);
if (__ret < 0) {
ret = component->driver->compr_ops->open(cstream);
if (ret < 0) {
dev_err(component->dev,
"Compress ASoC: can't open platform %s: %d\n",
component->name, __ret);
ret = __ret;
component->name, ret);
goto machine_err;
}
}
if (ret < 0)
goto machine_err;
component = NULL;

if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->startup) {
ret = rtd->dai_link->compr_ops->startup(cstream);
Expand All @@ -97,17 +96,20 @@ static int soc_compr_open(struct snd_compr_stream *cstream)

machine_err:
for_each_rtdcom(rtd, rtdcom) {
component = rtdcom->component;
struct snd_soc_component *err_comp = rtdcom->component;

if (err_comp == component)
break;

/* ignore duplication for now */
if (platform && (component == &platform->component))
if (platform && (err_comp == &platform->component))
continue;

if (!component->driver->compr_ops ||
!component->driver->compr_ops->free)
if (!err_comp->driver->compr_ops ||
!err_comp->driver->compr_ops->free)
continue;

component->driver->compr_ops->free(cstream);
err_comp->driver->compr_ops->free(cstream);
}

if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)
Expand All @@ -132,7 +134,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
struct snd_soc_dpcm *dpcm;
struct snd_soc_dapm_widget_list *list;
int stream;
int ret = 0, __ret;
int ret;

if (cstream->direction == SND_COMPRESS_PLAYBACK)
stream = SNDRV_PCM_STREAM_PLAYBACK;
Expand Down Expand Up @@ -172,16 +174,15 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
!component->driver->compr_ops->open)
continue;

__ret = component->driver->compr_ops->open(cstream);
if (__ret < 0) {
ret = component->driver->compr_ops->open(cstream);
if (ret < 0) {
dev_err(component->dev,
"Compress ASoC: can't open platform %s: %d\n",
component->name, __ret);
ret = __ret;
component->name, ret);
goto machine_err;
}
}
if (ret < 0)
goto machine_err;
component = NULL;

if (fe->dai_link->compr_ops && fe->dai_link->compr_ops->startup) {
ret = fe->dai_link->compr_ops->startup(cstream);
Expand Down Expand Up @@ -236,17 +237,20 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
fe->dai_link->compr_ops->shutdown(cstream);
machine_err:
for_each_rtdcom(fe, rtdcom) {
component = rtdcom->component;
struct snd_soc_component *err_comp = rtdcom->component;

if (err_comp == component)
break;

/* ignore duplication for now */
if (platform && (component == &platform->component))
if (platform && (err_comp == &platform->component))
continue;

if (!component->driver->compr_ops ||
!component->driver->compr_ops->free)
if (!err_comp->driver->compr_ops ||
!err_comp->driver->compr_ops->free)
continue;

component->driver->compr_ops->free(cstream);
err_comp->driver->compr_ops->free(cstream);
}

if (platform && platform->driver->compr_ops && platform->driver->compr_ops->free)
Expand Down

0 comments on commit 572e6c8

Please sign in to comment.