[PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core

[PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core

[Barry Song]

AC97 codec DAIs don't register themselves eg.stac9766, wm9712, wm9713, wm9705.c, ad1980,
then it loses to the chance to be given a null_dai_ops in snd_soc_register_dai if they
have no ops. When functions like soc_pcm_open, soc_pcm_hw_params etc. access the ops field
in these DAIs, panic will happen.

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 sound/soc/soc-core.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fa0da3c..6643d64 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -795,6 +795,9 @@ static int soc_resume(struct device *dev)
 #define soc_resume	NULL
 #endif
 
+static struct snd_soc_dai_ops null_dai_ops = {
+};
+
 static void snd_soc_instantiate_card(struct snd_soc_card *card)
 {
 	struct platform_device *pdev = container_of(card->dev,
@@ -838,6 +841,12 @@ static void snd_soc_instantiate_card(struct snd_soc_card *card)
 			ac97 = 1;
 	}
 
+	for (i = 0; i < card->num_links; i++) {
+		if (card->dai_link[i].codec_dai->ac97_control &&
+				(!card->dai_link[i].codec_dai->ops))
+			card->dai_link[i].codec_dai->ops = &null_dai_ops;
+	}
+
 	/* If we have AC97 in the system then don't wait for the
 	 * codec.  This will need revisiting if we have to handle
 	 * systems with mixed AC97 and non-AC97 parts.  Only check for
@@ -2332,9 +2341,6 @@ static int snd_soc_unregister_card(struct snd_soc_card *card)
 	return 0;
 }
 
-static struct snd_soc_dai_ops null_dai_ops = {
-};
-
 /**
  * snd_soc_register_dai - Register a DAI with the ASoC core
  *
-- 
1.5.6.3

Re: [PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core

[Mark Brown]

On Fri, Oct 16, 2009 at 10:09:20AM +0800, Barry Song wrote:
> AC97 codec DAIs don't register themselves eg.stac9766, wm9712, wm9713, wm9705.c, ad1980,
> then it loses to the chance to be given a null_dai_ops in snd_soc_register_dai if they
> have no ops. When functions like soc_pcm_open, soc_pcm_hw_params etc. access the ops field
> in these DAIs, panic will happen.

This is the wrong fix - null DAI ops are supposed to be supported for
all devices since not all DAIs will be configurable.

Re: [PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core

[Barry Song]

On Fri, Oct 16, 2009 at 5:08 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Oct 16, 2009 at 10:09:20AM +0800, Barry Song wrote:
>> AC97 codec DAIs don't register themselves eg.stac9766, wm9712, wm9713, wm9705.c, ad1980,
>> then it loses to the chance to be given a null_dai_ops in snd_soc_register_dai if they
>> have no ops. When functions like soc_pcm_open, soc_pcm_hw_params etc. access the ops field
>> in these DAIs, panic will happen.
>
> This is the wrong fix - null DAI ops are supposed to be supported for
> all devices since not all DAIs will be configurable.
But the problem is ac97 DAIs will have no path to get the null ops.
Then many functions will panic. It's very easy to repeat the problem
in your site, only by deleting ops field in any AC97 codec DAI.
>

Re: [PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core

[Mark Brown]

On Fri, Oct 16, 2009 at 05:14:31PM +0800, Barry Song wrote:
> On Fri, Oct 16, 2009 at 5:08 PM, Mark Brown

> > This is the wrong fix - null DAI ops are supposed to be supported for
> > all devices since not all DAIs will be configurable.

> But the problem is ac97 DAIs will have no path to get the null ops.
> Then many functions will panic. It's very easy to repeat the problem
> in your site, only by deleting ops field in any AC97 codec DAI.

Sure, I see the problem.  The issue is that this could affect any CODEC,
not just AC97 ones, so an AC97-specific fix isn't what's needed.  Given
the idiom of the rest of the code the best thing would be to make all
the remaining callers check for ops before dereferencing it.

Re: [PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core

[Barry Song]

On Fri, Oct 16, 2009 at 5:32 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Oct 16, 2009 at 05:14:31PM +0800, Barry Song wrote:
>> On Fri, Oct 16, 2009 at 5:08 PM, Mark Brown
>
>> > This is the wrong fix - null DAI ops are supposed to be supported for
>> > all devices since not all DAIs will be configurable.
>
>> But the problem is ac97 DAIs will have no path to get the null ops.
>> Then many functions will panic. It's very easy to repeat the problem
>> in your site, only by deleting ops field in any AC97 codec DAI.
>
> Sure, I see the problem.  The issue is that this could affect any CODEC,
> not just AC97 ones, so an AC97-specific fix isn't what's needed.  Given
> the idiom of the rest of the code the best thing would be to make all
> the remaining callers check for ops before dereferencing it.
At the beginning, I wanted to do like this. But I saw all other codec
DAIs register themselves, then got a null ops. Just ac97 is forgot.
Then I patched AC97 codec DAIs.
Then you prefer to patch all ops access?

>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core

[Mark Brown]

On Fri, Oct 16, 2009 at 05:35:12PM +0800, Barry Song wrote:

> At the beginning, I wanted to do like this. But I saw all other codec
> DAIs register themselves, then got a null ops. Just ac97 is forgot.
> Then I patched AC97 codec DAIs.
> Then you prefer to patch all ops access?

Hrm, so they do.  In that case could you revise your patch to not check
for AC97 but just check for null DAI ops instead since this isn't an
AC97 specific issue?