* [PATCH] SoC: cht_bsw_rt5645: Fix writing to string literal
@ 2016-02-15 21:36 Carlo Caione
2016-02-18 8:02 ` Pierre-Louis Bossart
0 siblings, 1 reply; 4+ messages in thread
From: Carlo Caione @ 2016-02-15 21:36 UTC (permalink / raw)
To: lgirdwood, broonie, vinod.koul, yang.a.fang, linux, alsa-devel
Cc: Carlo Caione
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) {
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] SoC: cht_bsw_rt5645: Fix writing to string literal
2016-02-15 21:36 [PATCH] SoC: cht_bsw_rt5645: Fix writing to string literal Carlo Caione
@ 2016-02-18 8:02 ` Pierre-Louis Bossart
2016-02-18 8:31 ` Carlo Caione
0 siblings, 1 reply; 4+ messages in thread
From: Pierre-Louis Bossart @ 2016-02-18 8:02 UTC (permalink / raw)
To: Carlo Caione, lgirdwood, broonie, vinod.koul, yang.a.fang, linux,
alsa-devel
Cc: Carlo Caione
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SoC: cht_bsw_rt5645: Fix writing to string literal
2016-02-18 8:02 ` Pierre-Louis Bossart
@ 2016-02-18 8:31 ` Carlo Caione
2016-02-20 15:01 ` Pierre-Louis Bossart
0 siblings, 1 reply; 4+ messages in thread
From: Carlo Caione @ 2016-02-18 8:31 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, vinod.koul, lgirdwood, yang.a.fang, Mark Brown,
Carlo Caione, Linux Upstreaming Team
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")?
--
Carlo Caione | +39.340.80.30.096 | Endless
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SoC: cht_bsw_rt5645: Fix writing to string literal
2016-02-18 8:31 ` Carlo Caione
@ 2016-02-20 15:01 ` Pierre-Louis Bossart
0 siblings, 0 replies; 4+ messages in thread
From: Pierre-Louis Bossart @ 2016-02-20 15:01 UTC (permalink / raw)
To: Carlo Caione
Cc: alsa-devel, vinod.koul, lgirdwood, yang.a.fang, Mark Brown,
Carlo Caione, Linux Upstreaming Team
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!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-20 15:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-15 21:36 [PATCH] SoC: cht_bsw_rt5645: Fix writing to string literal Carlo Caione
2016-02-18 8:02 ` Pierre-Louis Bossart
2016-02-18 8:31 ` Carlo Caione
2016-02-20 15:01 ` Pierre-Louis Bossart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.