[bug report] ASoC: Intel: cht_bsw_rt5645: Set card.components string
Hello Hans de Goede,
Commit f87b4402163b ("ASoC: Intel: cht_bsw_rt5645: Set card.components string") from Nov 26, 2023 (linux-next), leads to the following Smatch static checker warning:
sound/soc/intel/boards/cht_bsw_rt5645.c:587 snd_cht_mc_probe() error: we previously assumed 'adev' could be null (see line 581)
sound/soc/intel/boards/cht_bsw_rt5645.c 570 /* set correct codec name */ 571 for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) 572 if (cht_dailink[i].codecs->name && 573 !strcmp(cht_dailink[i].codecs->name, 574 "i2c-10EC5645:00")) { 575 dai_index = i; 576 break; 577 } 578 579 /* fixup codec name based on HID */ 580 adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1); 581 if (adev) { ^^^^ The old code assumes adev can be NULL
582 snprintf(cht_rt5645_codec_name, sizeof(cht_rt5645_codec_name), 583 "i2c-%s", acpi_dev_name(adev)); 584 cht_dailink[dai_index].codecs->name = cht_rt5645_codec_name; 585 } 586 /* acpi_get_first_physical_node() returns a borrowed ref, no need to deref */ --> 587 codec_dev = acpi_get_first_physical_node(adev); ^^^^ Unchecked dereference
588 acpi_dev_put(adev); 589 if (!codec_dev) 590 return -EPROBE_DEFER; 591 592 snd_soc_card_chtrt5645.components = rt5645_components(codec_dev); 593 snd_soc_card_chtrt5650.components = rt5645_components(codec_dev); 594
regards, dan carpenter
Thanks Dan for the report.
Commit f87b4402163b ("ASoC: Intel: cht_bsw_rt5645: Set card.components string") from Nov 26, 2023 (linux-next), leads to the following Smatch static checker warning:
sound/soc/intel/boards/cht_bsw_rt5645.c:587 snd_cht_mc_probe() error: we previously assumed 'adev' could be null (see line 581)
sound/soc/intel/boards/cht_bsw_rt5645.c 570 /* set correct codec name */ 571 for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) 572 if (cht_dailink[i].codecs->name && 573 !strcmp(cht_dailink[i].codecs->name, 574 "i2c-10EC5645:00")) { 575 dai_index = i; 576 break; 577 } 578 579 /* fixup codec name based on HID */ 580 adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1); 581 if (adev) { ^^^^ The old code assumes adev can be NULL
582 snprintf(cht_rt5645_codec_name, sizeof(cht_rt5645_codec_name), 583 "i2c-%s", acpi_dev_name(adev)); 584 cht_dailink[dai_index].codecs->name = cht_rt5645_codec_name; 585 } 586 /* acpi_get_first_physical_node() returns a borrowed ref, no need to deref */--> 587 codec_dev = acpi_get_first_physical_node(adev); ^^^^ Unchecked dereference
This looks like a problem in multiple machine drivers sharing similar code, if we want to consistently check the output we probably need something like https://github.com/thesofproject/linux/pull/5117 ?
On Mon, Jul 22, 2024 at 04:06:58PM +0200, Pierre-Louis Bossart wrote:
Thanks Dan for the report.
Commit f87b4402163b ("ASoC: Intel: cht_bsw_rt5645: Set card.components string") from Nov 26, 2023 (linux-next), leads to the following Smatch static checker warning:
sound/soc/intel/boards/cht_bsw_rt5645.c:587 snd_cht_mc_probe() error: we previously assumed 'adev' could be null (see line 581)
sound/soc/intel/boards/cht_bsw_rt5645.c 570 /* set correct codec name */ 571 for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) 572 if (cht_dailink[i].codecs->name && 573 !strcmp(cht_dailink[i].codecs->name, 574 "i2c-10EC5645:00")) { 575 dai_index = i; 576 break; 577 } 578 579 /* fixup codec name based on HID */ 580 adev = acpi_dev_get_first_match_dev(mach->id, NULL, -1); 581 if (adev) { ^^^^ The old code assumes adev can be NULL
582 snprintf(cht_rt5645_codec_name, sizeof(cht_rt5645_codec_name), 583 "i2c-%s", acpi_dev_name(adev)); 584 cht_dailink[dai_index].codecs->name = cht_rt5645_codec_name; 585 } 586 /* acpi_get_first_physical_node() returns a borrowed ref, no need to deref */--> 587 codec_dev = acpi_get_first_physical_node(adev); ^^^^ Unchecked dereference
This looks like a problem in multiple machine drivers sharing similar code, if we want to consistently check the output we probably need something like https://github.com/thesofproject/linux/pull/5117 ?
These are actually Smatch warnings, not Sparse warnings btw. (Smatch uses Sparse as a parser. Everyone gets them mixed up).
regards, dan carpenter
participants (2)
-
Dan Carpenter -
Pierre-Louis Bossart