[alsa-devel] [PATCH v2] ASoC: cht_bsw_rt5645: Fix writing to string literal
From: Carlo Caione carlo@endlessm.com
We cannot use strcpy() to write to a const char * location. This is causing a 'BUG: unable to handle kernel paging request' error at boot when using the cht-bsw-rt5645 driver.
With this patch we also fix a wrong indexing in the driver where the codec_name of the wrong dai_link is being overwritten.
Signed-off-by: Carlo Caione carlo@endlessm.com --- sound/soc/intel/boards/cht_bsw_rt5645.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index 2d3afdd..a7b96a9 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -367,8 +367,12 @@ static int snd_cht_mc_probe(struct platform_device *pdev) } card->dev = &pdev->dev; sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id); + /* set correct codec name */ - strcpy((char *)card->dai_link[2].codec_name, codec_name); + for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) + if (!strcmp(card->dai_link[i].codec_name, "i2c-10EC5645:00")) + card->dai_link[i].codec_name = kstrdup(codec_name, GFP_KERNEL); + snd_soc_card_set_drvdata(card, drv); ret_val = devm_snd_soc_register_card(&pdev->dev, card); if (ret_val) {
On Tue, Feb 23, 2016 at 09:50:20AM +0100, Carlo Caione wrote:
From: Carlo Caione carlo@endlessm.com
We cannot use strcpy() to write to a const char * location. This is causing a 'BUG: unable to handle kernel paging request' error at boot when using the cht-bsw-rt5645 driver.
With this patch we also fix a wrong indexing in the driver where the codec_name of the wrong dai_link is being overwritten.
So how was the original code tested then...?
On Fri, Feb 26, 2016 at 11:12:10AM +0900, Mark Brown wrote:
On Tue, Feb 23, 2016 at 09:50:20AM +0100, Carlo Caione wrote:
From: Carlo Caione carlo@endlessm.com
We cannot use strcpy() to write to a const char * location. This is causing a 'BUG: unable to handle kernel paging request' error at boot when using the cht-bsw-rt5645 driver.
With this patch we also fix a wrong indexing in the driver where the codec_name of the wrong dai_link is being overwritten.
So how was the original code tested then...?
I know systems using this are shipping so surprised to see this.
Carlo cna you please tell me the configuration where it is seen.
Also would be worth mentioning here that Pierre is working on a super fix for this which involves removing this code and getting the right IDs from BIOS, so we wont need this code.
Thanks
On Thu, Mar 3, 2016 at 12:32 PM, Vinod Koul vinod.koul@intel.com wrote:
On Fri, Feb 26, 2016 at 11:12:10AM +0900, Mark Brown wrote:
On Tue, Feb 23, 2016 at 09:50:20AM +0100, Carlo Caione wrote:
From: Carlo Caione carlo@endlessm.com
We cannot use strcpy() to write to a const char * location. This is causing a 'BUG: unable to handle kernel paging request' error at boot when using the cht-bsw-rt5645 driver.
With this patch we also fix a wrong indexing in the driver where the codec_name of the wrong dai_link is being overwritten.
So how was the original code tested then...?
I know systems using this are shipping so surprised to see this.
Carlo cna you please tell me the configuration where it is seen.
Hi Vinod, I'm working on the ECS EF20EA cherry-trail laptop. This is the used configuration:
https://gist.github.com/carlocaione/925540bc7b04eff01aa8
Cheers,
On 3/3/16 5:32 AM, Vinod Koul wrote:
On Fri, Feb 26, 2016 at 11:12:10AM +0900, Mark Brown wrote:
On Tue, Feb 23, 2016 at 09:50:20AM +0100, Carlo Caione wrote:
From: Carlo Caione carlo@endlessm.com
We cannot use strcpy() to write to a const char * location. This is causing a 'BUG: unable to handle kernel paging request' error at boot when using the cht-bsw-rt5645 driver.
With this patch we also fix a wrong indexing in the driver where the codec_name of the wrong dai_link is being overwritten.
So how was the original code tested then...?
I know systems using this are shipping so surprised to see this.
Carlo cna you please tell me the configuration where it is seen.
Also would be worth mentioning here that Pierre is working on a super fix for this which involves removing this code and getting the right IDs from BIOS, so we wont need this code.
I am actually using the same code to find out which DAI fields need to be replaced instead of hard-codec indices so the loop part is a good thing. There is still a need to copy a string, be it a constant to generated from the HID, into the codec_name field. I am not clear why strcpy() works on all my systems and why kstrdup() is required. Is the kernel issue really due to this string handling or to the bad index?
On Thu, Mar 3, 2016 at 5:11 PM, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 3/3/16 5:32 AM, Vinod Koul wrote:
On Fri, Feb 26, 2016 at 11:12:10AM +0900, Mark Brown wrote:
On Tue, Feb 23, 2016 at 09:50:20AM +0100, Carlo Caione wrote:
From: Carlo Caione carlo@endlessm.com
We cannot use strcpy() to write to a const char * location. This is causing a 'BUG: unable to handle kernel paging request' error at boot when using the cht-bsw-rt5645 driver.
With this patch we also fix a wrong indexing in the driver where the codec_name of the wrong dai_link is being overwritten.
So how was the original code tested then...?
I know systems using this are shipping so surprised to see this.
Carlo cna you please tell me the configuration where it is seen.
Also would be worth mentioning here that Pierre is working on a super fix for this which involves removing this code and getting the right IDs from BIOS, so we wont need this code.
I am actually using the same code to find out which DAI fields need to be replaced instead of hard-codec indices so the loop part is a good thing. There is still a need to copy a string, be it a constant to generated from the HID, into the codec_name field. I am not clear why strcpy() works on all my systems and why kstrdup() is required. Is the kernel issue really due to this string handling or to the bad index?
In my tests using kstrdup() fixed the BUG issue and the bad indexing fix was required to have the driver working correctly.
The patch
ASoC: cht_bsw_rt5645: Fix writing to string literal
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From c8560b7c917f8738f5d80dd516930edc1d05e4e4 Mon Sep 17 00:00:00 2001
From: Carlo Caione carlo@endlessm.com Date: Tue, 23 Feb 2016 09:50:20 +0100 Subject: [PATCH] ASoC: cht_bsw_rt5645: Fix writing to string literal
We cannot use strcpy() to write to a const char * location. This is causing a 'BUG: unable to handle kernel paging request' error at boot when using the cht-bsw-rt5645 driver.
With this patch we also fix a wrong indexing in the driver where the codec_name of the wrong dai_link is being overwritten.
Signed-off-by: Carlo Caione carlo@endlessm.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/cht_bsw_rt5645.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index 2d3afddb0a2e..a7b96a9a4e0e 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -367,8 +367,12 @@ static int snd_cht_mc_probe(struct platform_device *pdev) } card->dev = &pdev->dev; sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id); + /* set correct codec name */ - strcpy((char *)card->dai_link[2].codec_name, codec_name); + for (i = 0; i < ARRAY_SIZE(cht_dailink); i++) + if (!strcmp(card->dai_link[i].codec_name, "i2c-10EC5645:00")) + card->dai_link[i].codec_name = kstrdup(codec_name, GFP_KERNEL); + snd_soc_card_set_drvdata(card, drv); ret_val = devm_snd_soc_register_card(&pdev->dev, card); if (ret_val) {
participants (5)
-
Carlo Caione -
Carlo Caione -
Mark Brown -
Pierre-Louis Bossart -
Vinod Koul