[alsa-devel] [PATCH] SoC: 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/atom/sst-atom-controls.h | 1 + sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/atom/sst-atom-controls.h b/sound/soc/intel/atom/sst-atom-controls.h index e011311..6f88d1c 100644 --- a/sound/soc/intel/atom/sst-atom-controls.h +++ b/sound/soc/intel/atom/sst-atom-controls.h @@ -30,6 +30,7 @@ enum { MERR_DPCM_AUDIO = 0, MERR_DPCM_DEEP_BUFFER, MERR_DPCM_COMPR, + MERR_DPCM_BE, };
/* define a bit for each mixer input */ diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index e6cf800..89b4c032 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { }, /* CODEC<->CODEC link */ /* back ends */ - { + [MERR_DPCM_BE] = { .name = "SSP2-Codec", .be_id = 1, .cpu_dai_name = "ssp2-port", @@ -357,7 +357,7 @@ 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); + card->dai_link[MERR_DPCM_BE].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 2/15/16 1:36 PM, 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.
Signed-off-by: Carlo Caione carlo@endlessm.com
sound/soc/intel/atom/sst-atom-controls.h | 1 + sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/atom/sst-atom-controls.h b/sound/soc/intel/atom/sst-atom-controls.h index e011311..6f88d1c 100644 --- a/sound/soc/intel/atom/sst-atom-controls.h +++ b/sound/soc/intel/atom/sst-atom-controls.h @@ -30,6 +30,7 @@ enum { MERR_DPCM_AUDIO = 0, MERR_DPCM_DEEP_BUFFER, MERR_DPCM_COMPR,
MERR_DPCM_BE, };
/* define a bit for each mixer input */
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index e6cf800..89b4c032 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { }, /* CODEC<->CODEC link */ /* back ends */
- {
- [MERR_DPCM_BE] = { .name = "SSP2-Codec", .be_id = 1, .cpu_dai_name = "ssp2-port",
@@ -357,7 +357,7 @@ 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);
- card->dai_link[MERR_DPCM_BE].codec_name = kstrdup(codec_name, GFP_KERNEL);
It's probably less problematic to find the index in a different way. Adding a new definition in the platform driver doesn't seem quite right.
On Thu, Feb 18, 2016 at 9:02 AM, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 2/15/16 1:36 PM, 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.
Signed-off-by: Carlo Caione carlo@endlessm.com
sound/soc/intel/atom/sst-atom-controls.h | 1 + sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/atom/sst-atom-controls.h b/sound/soc/intel/atom/sst-atom-controls.h index e011311..6f88d1c 100644 --- a/sound/soc/intel/atom/sst-atom-controls.h +++ b/sound/soc/intel/atom/sst-atom-controls.h @@ -30,6 +30,7 @@ enum { MERR_DPCM_AUDIO = 0, MERR_DPCM_DEEP_BUFFER, MERR_DPCM_COMPR,
MERR_DPCM_BE,};
/* define a bit for each mixer input */
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index e6cf800..89b4c032 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { }, /* CODEC<->CODEC link */ /* back ends */
{
[MERR_DPCM_BE] = { .name = "SSP2-Codec", .be_id = 1, .cpu_dai_name = "ssp2-port",@@ -357,7 +357,7 @@ 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);
card->dai_link[MERR_DPCM_BE].codec_name = kstrdup(codec_name,GFP_KERNEL);
It's probably less problematic to find the index in a different way. Adding a new definition in the platform driver doesn't seem quite right.
MERR_DPCM_BE is intended to be the last dai link index corresponding to the BE dai. Or at least that was my idea. What about finding the index using a cycle and stopping on !strcmp(card->dai_link[i].codec_name, "i2c-10EC5645:00")?
On 02/18/2016 02:31 AM, Carlo Caione wrote:
On Thu, Feb 18, 2016 at 9:02 AM, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 2/15/16 1:36 PM, 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.
Signed-off-by: Carlo Caione carlo@endlessm.com
sound/soc/intel/atom/sst-atom-controls.h | 1 + sound/soc/intel/boards/cht_bsw_rt5645.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/intel/atom/sst-atom-controls.h b/sound/soc/intel/atom/sst-atom-controls.h index e011311..6f88d1c 100644 --- a/sound/soc/intel/atom/sst-atom-controls.h +++ b/sound/soc/intel/atom/sst-atom-controls.h @@ -30,6 +30,7 @@ enum { MERR_DPCM_AUDIO = 0, MERR_DPCM_DEEP_BUFFER, MERR_DPCM_COMPR,
MERR_DPCM_BE,};
/* define a bit for each mixer input */
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index e6cf800..89b4c032 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -282,7 +282,7 @@ static struct snd_soc_dai_link cht_dailink[] = { }, /* CODEC<->CODEC link */ /* back ends */
{
[MERR_DPCM_BE] = { .name = "SSP2-Codec", .be_id = 1, .cpu_dai_name = "ssp2-port",@@ -357,7 +357,7 @@ 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);
card->dai_link[MERR_DPCM_BE].codec_name = kstrdup(codec_name,GFP_KERNEL);
It's probably less problematic to find the index in a different way. Adding a new definition in the platform driver doesn't seem quite right.
MERR_DPCM_BE is intended to be the last dai link index corresponding to the BE dai. Or at least that was my idea. What about finding the index using a cycle and stopping one !strcmp(card->dai_link[i].codec_name, "i2c-10EC5645:00")?
Yes that was the idea. My concern is that if someone adds a front-end or conversely does not enable the link in the machine driver the index will be off. btw we have a fix coming to change the codec name, in some cases the BIOS is somewhat broken and the extension can be :01. If you go ahead with the loop I'll add the changes on top of yours in a follow-up patch. Thanks!
participants (3)
-
Carlo Caione -
Carlo Caione -
Pierre-Louis Bossart