[alsa-devel] [PATCH] Fix codec_dai->ops NULL pointer bug for AC97 codec dai in soc-core
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 *
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.
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.
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.
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?
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?
participants (2)
-
Barry Song -
Mark Brown