Re: [PATCH 6.11 regression fix] ASoC: Intel: Boards: Fix NULL pointer deref in BYT/CHT boards harder

[Hans de Goede <hdegoede@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxx
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  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;