| Message ID | 20240823074217.14653-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0cc65482f5b03ac2b1c240bc34665e43ea2d71bb |
| Headers | show |
| Series | [6.11,regression,fix] ASoC: Intel: Boards: Fix NULL pointer deref in BYT/CHT boards harder | expand |
On 8/23/24 09:42, Hans de Goede wrote: > Since commit 13f58267cda3 ("ASoC: soc.h: don't create dummy Component > via COMP_DUMMY()") dummy codecs declared like this: > > SND_SOC_DAILINK_DEF(dummy, > DAILINK_COMP_ARRAY(COMP_DUMMY())); > > expand to: > > static struct snd_soc_dai_link_component dummy[] = { > }; > > Which means that dummy is a zero sized array and thus dais[i].codecs should > not be dereferenced *at all* since it points to the address of the next > variable stored in the data section as the "dummy" variable has an address > but no size, so even dereferencing dais[0] is already an out of bounds > array reference. > > Which means that the if (dais[i].codecs->name) check added in > commit 7d99a70b6595 ("ASoC: Intel: Boards: Fix NULL pointer deref > in BYT/CHT boards") relies on that the part of the next variable which > the name member maps to just happens to be NULL. > > Which apparently so far it usually is, except when it isn't > and then it results in crashes like this one: > > [ 28.795659] BUG: unable to handle page fault for address: 0000000000030011 > ... > [ 28.795780] Call Trace: > [ 28.795787] <TASK> > ... > [ 28.795862] ? strcmp+0x18/0x40 > [ 28.795872] 0xffffffffc150c605 > [ 28.795887] platform_probe+0x40/0xa0 > ... > [ 28.795979] ? __pfx_init_module+0x10/0x10 [snd_soc_sst_bytcr_wm5102] > > Really fix things this time around by checking dais.num_codecs != 0. > > Fixes: 7d99a70b6595 ("ASoC: Intel: Boards: Fix NULL pointer deref in BYT/CHT boards") > Cc: stable@vger.kernel.org > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Hi, Note the subject is not entirely correct, I first noticed / hit this with 6.11, but I believe it is present in older kernels too. Either way the important thing is that this is a regression fix and should be send to Linus this cycle. Regards, Hans On 8/23/24 9:42 AM, Hans de Goede wrote: > Since commit 13f58267cda3 ("ASoC: soc.h: don't create dummy Component > via COMP_DUMMY()") dummy codecs declared like this: > > SND_SOC_DAILINK_DEF(dummy, > DAILINK_COMP_ARRAY(COMP_DUMMY())); > > expand to: > > static struct snd_soc_dai_link_component dummy[] = { > }; > > Which means that dummy is a zero sized array and thus dais[i].codecs should > not be dereferenced *at all* since it points to the address of the next > variable stored in the data section as the "dummy" variable has an address > but no size, so even dereferencing dais[0] is already an out of bounds > array reference. > > Which means that the if (dais[i].codecs->name) check added in > commit 7d99a70b6595 ("ASoC: Intel: Boards: Fix NULL pointer deref > in BYT/CHT boards") relies on that the part of the next variable which > the name member maps to just happens to be NULL. > > Which apparently so far it usually is, except when it isn't > and then it results in crashes like this one: > > [ 28.795659] BUG: unable to handle page fault for address: 0000000000030011 > ... > [ 28.795780] Call Trace: > [ 28.795787] <TASK> > ... > [ 28.795862] ? strcmp+0x18/0x40 > [ 28.795872] 0xffffffffc150c605 > [ 28.795887] platform_probe+0x40/0xa0 > ... > [ 28.795979] ? __pfx_init_module+0x10/0x10 [snd_soc_sst_bytcr_wm5102] > > Really fix things this time around by checking dais.num_codecs != 0. > > Fixes: 7d99a70b6595 ("ASoC: Intel: Boards: Fix NULL pointer deref in BYT/CHT boards") > Cc: stable@vger.kernel.org > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > sound/soc/intel/boards/bxt_rt298.c | 2 +- > sound/soc/intel/boards/bytcht_cx2072x.c | 2 +- > sound/soc/intel/boards/bytcht_da7213.c | 2 +- > sound/soc/intel/boards/bytcht_es8316.c | 2 +- > sound/soc/intel/boards/bytcr_rt5640.c | 2 +- > sound/soc/intel/boards/bytcr_rt5651.c | 2 +- > sound/soc/intel/boards/bytcr_wm5102.c | 2 +- > sound/soc/intel/boards/cht_bsw_rt5645.c | 2 +- > sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +- > 9 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/sound/soc/intel/boards/bxt_rt298.c b/sound/soc/intel/boards/bxt_rt298.c > index dce6a2086f2a..6da1517c53c6 100644 > --- a/sound/soc/intel/boards/bxt_rt298.c > +++ b/sound/soc/intel/boards/bxt_rt298.c > @@ -605,7 +605,7 @@ static int broxton_audio_probe(struct platform_device *pdev) > int i; > > for (i = 0; i < ARRAY_SIZE(broxton_rt298_dais); i++) { > - if (card->dai_link[i].codecs->name && > + if (card->dai_link[i].num_codecs && > !strncmp(card->dai_link[i].codecs->name, "i2c-INT343A:00", > I2C_NAME_SIZE)) { > if (!strncmp(card->name, "broxton-rt298", > diff --git a/sound/soc/intel/boards/bytcht_cx2072x.c b/sound/soc/intel/boards/bytcht_cx2072x.c > index c014d85a08b2..df3c2a7b64d2 100644 > --- a/sound/soc/intel/boards/bytcht_cx2072x.c > +++ b/sound/soc/intel/boards/bytcht_cx2072x.c > @@ -241,7 +241,7 @@ static int snd_byt_cht_cx2072x_probe(struct platform_device *pdev) > > /* fix index of codec dai */ > for (i = 0; i < ARRAY_SIZE(byt_cht_cx2072x_dais); i++) { > - if (byt_cht_cx2072x_dais[i].codecs->name && > + if (byt_cht_cx2072x_dais[i].num_codecs && > !strcmp(byt_cht_cx2072x_dais[i].codecs->name, > "i2c-14F10720:00")) { > dai_index = i; > diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c > index f4ac3ddd148b..08c598b7e1ee 100644 > --- a/sound/soc/intel/boards/bytcht_da7213.c > +++ b/sound/soc/intel/boards/bytcht_da7213.c > @@ -245,7 +245,7 @@ static int bytcht_da7213_probe(struct platform_device *pdev) > > /* fix index of codec dai */ > for (i = 0; i < ARRAY_SIZE(dailink); i++) { > - if (dailink[i].codecs->name && > + if (dailink[i].num_codecs && > !strcmp(dailink[i].codecs->name, "i2c-DLGS7213:00")) { > dai_index = i; > break; > diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c > index 2fcec2e02bb5..77b91ea4dc32 100644 > --- a/sound/soc/intel/boards/bytcht_es8316.c > +++ b/sound/soc/intel/boards/bytcht_es8316.c > @@ -546,7 +546,7 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev) > > /* fix index of codec dai */ > for (i = 0; i < ARRAY_SIZE(byt_cht_es8316_dais); i++) { > - if (byt_cht_es8316_dais[i].codecs->name && > + if (byt_cht_es8316_dais[i].num_codecs && > !strcmp(byt_cht_es8316_dais[i].codecs->name, > "i2c-ESSX8316:00")) { > dai_index = i; > diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c > index a64d1989e28a..db4a33680d94 100644 > --- a/sound/soc/intel/boards/bytcr_rt5640.c > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > @@ -1677,7 +1677,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) > > /* fix index of codec dai */ > for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) { > - if (byt_rt5640_dais[i].codecs->name && > + if (byt_rt5640_dais[i].num_codecs && > !strcmp(byt_rt5640_dais[i].codecs->name, > "i2c-10EC5640:00")) { > dai_index = i; > diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c > index 80c841b000a3..8514b79f389b 100644 > --- a/sound/soc/intel/boards/bytcr_rt5651.c > +++ b/sound/soc/intel/boards/bytcr_rt5651.c > @@ -910,7 +910,7 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev) > > /* fix index of codec dai */ > for (i = 0; i < ARRAY_SIZE(byt_rt5651_dais); i++) { > - if (byt_rt5651_dais[i].codecs->name && > + if (byt_rt5651_dais[i].num_codecs && > !strcmp(byt_rt5651_dais[i].codecs->name, > "i2c-10EC5651:00")) { > dai_index = i; > diff --git a/sound/soc/intel/boards/bytcr_wm5102.c b/sound/soc/intel/boards/bytcr_wm5102.c > index cccb5e90c0fe..e5a7cc606aa9 100644 > --- a/sound/soc/intel/boards/bytcr_wm5102.c > +++ b/sound/soc/intel/boards/bytcr_wm5102.c > @@ -605,7 +605,7 @@ static int snd_byt_wm5102_mc_probe(struct platform_device *pdev) > > /* find index of codec dai */ > for (i = 0; i < ARRAY_SIZE(byt_wm5102_dais); i++) { > - if (byt_wm5102_dais[i].codecs->name && > + if (byt_wm5102_dais[i].num_codecs && > !strcmp(byt_wm5102_dais[i].codecs->name, > "wm5102-codec")) { > dai_index = i; > diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c > index eb41b7115d01..1da9ceee4d59 100644 > --- a/sound/soc/intel/boards/cht_bsw_rt5645.c > +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c > @@ -569,7 +569,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) > > /* set correct codec name */ > for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) > - if (cht_dailink[i].codecs->name && > + if (cht_dailink[i].num_codecs && > !strcmp(cht_dailink[i].codecs->name, > "i2c-10EC5645:00")) { > dai_index = i; > diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c > index be2d1a8dbca8..d68e5bc755de 100644 > --- a/sound/soc/intel/boards/cht_bsw_rt5672.c > +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c > @@ -466,7 +466,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) > > /* find index of codec dai */ > for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) { > - if (cht_dailink[i].codecs->name && > + if (cht_dailink[i].num_codecs && > !strcmp(cht_dailink[i].codecs->name, RT5672_I2C_DEFAULT)) { > dai_index = i; > break;
On Fri, 23 Aug 2024 09:42:17 +0200, Hans de Goede wrote: > Since commit 13f58267cda3 ("ASoC: soc.h: don't create dummy Component > via COMP_DUMMY()") dummy codecs declared like this: > > SND_SOC_DAILINK_DEF(dummy, > DAILINK_COMP_ARRAY(COMP_DUMMY())); > > expand to: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: Intel: Boards: Fix NULL pointer deref in BYT/CHT boards harder commit: 0cc65482f5b03ac2b1c240bc34665e43ea2d71bb All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/sound/soc/intel/boards/bxt_rt298.c b/sound/soc/intel/boards/bxt_rt298.c index dce6a2086f2a..6da1517c53c6 100644 --- a/sound/soc/intel/boards/bxt_rt298.c +++ b/sound/soc/intel/boards/bxt_rt298.c @@ -605,7 +605,7 @@ static int broxton_audio_probe(struct platform_device *pdev) int i; for (i = 0; i < ARRAY_SIZE(broxton_rt298_dais); i++) { - if (card->dai_link[i].codecs->name && + if (card->dai_link[i].num_codecs && !strncmp(card->dai_link[i].codecs->name, "i2c-INT343A:00", I2C_NAME_SIZE)) { if (!strncmp(card->name, "broxton-rt298", diff --git a/sound/soc/intel/boards/bytcht_cx2072x.c b/sound/soc/intel/boards/bytcht_cx2072x.c index c014d85a08b2..df3c2a7b64d2 100644 --- a/sound/soc/intel/boards/bytcht_cx2072x.c +++ b/sound/soc/intel/boards/bytcht_cx2072x.c @@ -241,7 +241,7 @@ static int snd_byt_cht_cx2072x_probe(struct platform_device *pdev) /* fix index of codec dai */ for (i = 0; i < ARRAY_SIZE(byt_cht_cx2072x_dais); i++) { - if (byt_cht_cx2072x_dais[i].codecs->name && + if (byt_cht_cx2072x_dais[i].num_codecs && !strcmp(byt_cht_cx2072x_dais[i].codecs->name, "i2c-14F10720:00")) { dai_index = i; diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c index f4ac3ddd148b..08c598b7e1ee 100644 --- a/sound/soc/intel/boards/bytcht_da7213.c +++ b/sound/soc/intel/boards/bytcht_da7213.c @@ -245,7 +245,7 @@ static int bytcht_da7213_probe(struct platform_device *pdev) /* fix index of codec dai */ for (i = 0; i < ARRAY_SIZE(dailink); i++) { - if (dailink[i].codecs->name && + if (dailink[i].num_codecs && !strcmp(dailink[i].codecs->name, "i2c-DLGS7213:00")) { dai_index = i; break; diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c index 2fcec2e02bb5..77b91ea4dc32 100644 --- a/sound/soc/intel/boards/bytcht_es8316.c +++ b/sound/soc/intel/boards/bytcht_es8316.c @@ -546,7 +546,7 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev) /* fix index of codec dai */ for (i = 0; i < ARRAY_SIZE(byt_cht_es8316_dais); i++) { - if (byt_cht_es8316_dais[i].codecs->name && + if (byt_cht_es8316_dais[i].num_codecs && !strcmp(byt_cht_es8316_dais[i].codecs->name, "i2c-ESSX8316:00")) { dai_index = i; diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index a64d1989e28a..db4a33680d94 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -1677,7 +1677,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) /* fix index of codec dai */ for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) { - if (byt_rt5640_dais[i].codecs->name && + if (byt_rt5640_dais[i].num_codecs && !strcmp(byt_rt5640_dais[i].codecs->name, "i2c-10EC5640:00")) { dai_index = i; diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index 80c841b000a3..8514b79f389b 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -910,7 +910,7 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev) /* fix index of codec dai */ for (i = 0; i < ARRAY_SIZE(byt_rt5651_dais); i++) { - if (byt_rt5651_dais[i].codecs->name && + if (byt_rt5651_dais[i].num_codecs && !strcmp(byt_rt5651_dais[i].codecs->name, "i2c-10EC5651:00")) { dai_index = i; diff --git a/sound/soc/intel/boards/bytcr_wm5102.c b/sound/soc/intel/boards/bytcr_wm5102.c index cccb5e90c0fe..e5a7cc606aa9 100644 --- a/sound/soc/intel/boards/bytcr_wm5102.c +++ b/sound/soc/intel/boards/bytcr_wm5102.c @@ -605,7 +605,7 @@ static int snd_byt_wm5102_mc_probe(struct platform_device *pdev) /* find index of codec dai */ for (i = 0; i < ARRAY_SIZE(byt_wm5102_dais); i++) { - if (byt_wm5102_dais[i].codecs->name && + if (byt_wm5102_dais[i].num_codecs && !strcmp(byt_wm5102_dais[i].codecs->name, "wm5102-codec")) { dai_index = i; diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index eb41b7115d01..1da9ceee4d59 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -569,7 +569,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) /* set correct codec name */ for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) - if (cht_dailink[i].codecs->name && + if (cht_dailink[i].num_codecs && !strcmp(cht_dailink[i].codecs->name, "i2c-10EC5645:00")) { dai_index = i; diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c index be2d1a8dbca8..d68e5bc755de 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5672.c +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c @@ -466,7 +466,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) /* find index of codec dai */ for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) { - if (cht_dailink[i].codecs->name && + if (cht_dailink[i].num_codecs && !strcmp(cht_dailink[i].codecs->name, RT5672_I2C_DEFAULT)) { dai_index = i; break;
Since commit 13f58267cda3 ("ASoC: soc.h: don't create dummy Component via COMP_DUMMY()") dummy codecs declared like this: SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY())); expand to: static struct snd_soc_dai_link_component dummy[] = { }; Which means that dummy is a zero sized array and thus dais[i].codecs should not be dereferenced *at all* since it points to the address of the next variable stored in the data section as the "dummy" variable has an address but no size, so even dereferencing dais[0] is already an out of bounds array reference. Which means that the if (dais[i].codecs->name) check added in commit 7d99a70b6595 ("ASoC: Intel: Boards: Fix NULL pointer deref in BYT/CHT boards") relies on that the part of the next variable which the name member maps to just happens to be NULL. Which apparently so far it usually is, except when it isn't and then it results in crashes like this one: [ 28.795659] BUG: unable to handle page fault for address: 0000000000030011 ... [ 28.795780] Call Trace: [ 28.795787] <TASK> ... [ 28.795862] ? strcmp+0x18/0x40 [ 28.795872] 0xffffffffc150c605 [ 28.795887] platform_probe+0x40/0xa0 ... [ 28.795979] ? __pfx_init_module+0x10/0x10 [snd_soc_sst_bytcr_wm5102] Really fix things this time around by checking dais.num_codecs != 0. Fixes: 7d99a70b6595 ("ASoC: Intel: Boards: Fix NULL pointer deref in BYT/CHT boards") Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- sound/soc/intel/boards/bxt_rt298.c | 2 +- sound/soc/intel/boards/bytcht_cx2072x.c | 2 +- sound/soc/intel/boards/bytcht_da7213.c | 2 +- sound/soc/intel/boards/bytcht_es8316.c | 2 +- sound/soc/intel/boards/bytcr_rt5640.c | 2 +- sound/soc/intel/boards/bytcr_rt5651.c | 2 +- sound/soc/intel/boards/bytcr_wm5102.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5645.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-)