[alsa-devel] Question about ASOC codec drivers
Hi,
Im trying to write an ASOC Codec driver for a new codec and im having problems setting up the widgets and interconnects.
The input structure is similar to the wm8731 so i have based my codec on this (my widget code is included at the bottom)
problem is I get this error on startup:
Failed to add route Line Input->Input Mux
Add then when I run alsa-mixer I get a memfault in the strcpy in snd_soc_info_enum_double()
it appears to be indexing a string 1 past the end, ie uinfo->value.enumerated.item = 3 but e->texts is pointing to the tansen_input_select[] array of strings which only has 3 entries, so it blows up (e->max = 7??).
also shouldn't the strcpy by a strncpy as we are copying to a fixed 64byte buffer ?
thanks for anyhelp.
Neil
my widget code:
static const struct snd_kcontrol_new tansen_snd_controls[] = { SOC_DOUBLE_R("Master Playback Volume", TANSEN_TM_OP_TOP7, TANSEN_TM_OP_TOP8, 0, 0xFF, 0 ), SOC_DOUBLE("Line/IPOD-In Gain", TANSEN_TM_IP_TOP5, 5, 2, 0x7, 0), SOC_DOUBLE("Mic-In Gain", TANSEN_TM_IP_TOP4, 0, 4, 0xF, 0), };
static const char *tansen_input_select[] = { "Mic In", "Line In", "IPOD In", };
static const struct soc_enum tansen_ip_enum = SOC_ENUM_SINGLE(TANSEN_TM_IP_TOP2, 4, 0x7, tansen_input_select);
/* Input mux */ static const struct snd_kcontrol_new tansen_input_mux_controls = SOC_DAPM_ENUM("Input Select", tansen_ip_enum);
static const struct snd_soc_dapm_widget tansen_dapm_widgets[] = { SND_SOC_DAPM_DAC("DAC", "Playback", SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_OUTPUT("LHPOUT"), SND_SOC_DAPM_OUTPUT("RHPOUT"), SND_SOC_DAPM_ADC("ADC", "Capture", SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_MUX("Input Mux", SND_SOC_NOPM, 0, 0, &tansen_input_mux_controls), SND_SOC_DAPM_PGA("PGA", SND_SOC_NOPM, 0, 0, NULL, 0), SND_SOC_DAPM_MICBIAS("Mic Bias", TANSEN_TM_MICBIAS_1, 4, 1), SND_SOC_DAPM_INPUT("MICIN"), SND_SOC_DAPM_INPUT("RLINEIN"), SND_SOC_DAPM_INPUT("LLINEIN"), SND_SOC_DAPM_INPUT("RIPODIN"), SND_SOC_DAPM_INPUT("LIPODIN") };
static const struct snd_soc_dapm_route intercon[] = {
/* outputs */ {"RHPOUT", NULL, "DAC"}, {"LHPOUT", NULL, "DAC"},
/* input mux */ {"Input Mux", "Line In", "Line Input"}, {"Input Mux", "Mic In", "Mic bias"}, {"Input Mux", "Ipod In", "Ipod Input"}, {"ADC", NULL, "PGA"}, {"PGA", NULL, "Input Mux"},
/* inputs */ {"Line Input", NULL, "LLINEIN"}, {"Line Input", NULL, "RLINEIN"}, {"Ipod Input", NULL, "LIPODIN"}, {"Ipod Input", NULL, "RIPODIN"}, {"Mic bias", NULL, "MICIN"}, };
On Tue, Jan 18, 2011 at 05:35:22PM +0000, Neil Jones wrote:
tansen_input_select[] array of strings which only has 3 entries, so it blows up (e->max = 7??).
This...
also shouldn't the strcpy by a strncpy as we are copying to a fixed 64byte buffer ?
There's rather a lot of strcpy() calls in the kernel; perhaps you could be more specific? Ideally if you think there's a change that should be made please post a patch.
static const struct soc_enum tansen_ip_enum = SOC_ENUM_SINGLE(TANSEN_TM_IP_TOP2, 4, 0x7, tansen_input_select);
...is exactly what you told the driver to do. You've told the core there are seven items in the enumeration so it's expecting an array of seven strings.
...is exactly what you told the driver to do. You've told the core there are seven items in the enumeration so it's expecting an array of seven strings.
DUH! sorry, in my head i read .xmax as .xmask cheers for the spot.
I still get 'Failed to add route Line Input->Input Mux' though ?
Ideally if you think there's a change that should be made please post a patch.
From: Neil Jones neiljay@gmail.com Date: Thu, 20 Jan 2011 15:59:38 +0000 Subject: [PATCH] sound/soc: change potentially risky strcpy to strncpy.
Signed-off-by: Neil Jones neiljay@gmail.com --- include/sound/asound.h | 11 +++++++---- sound/soc/soc-core.c | 5 +++-- 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/sound/asound.h b/include/sound/asound.h index a1803ec..f694604 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -761,6 +761,8 @@ typedef int __bitwise snd_ctl_elem_iface_t; #define SNDRV_CTL_POWER_D3hot (SNDRV_CTL_POWER_D3|0x0000) /* Off, with power */ #define SNDRV_CTL_POWER_D3cold (SNDRV_CTL_POWER_D3|0x0001) /* Off, without power */
+#define SNDRV_CTL_ELEMENT_INFO_NAME_LENGTH 64 + struct snd_ctl_elem_id { unsigned int numid; /* numeric identifier, zero = invalid */ snd_ctl_elem_iface_t iface; /* interface identifier */ @@ -799,7 +801,8 @@ struct snd_ctl_elem_info { struct { unsigned int items; /* R: number of items */ unsigned int item; /* W: item number */ - char name[64]; /* R: value name */ + /* R: value name */ + char name[SNDRV_CTL_ELEMENT_INFO_NAME_LENGTH]; } enumerated; unsigned char reserved[128]; } value; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 85b7d54..3ad3361 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2056,8 +2056,9 @@ int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
if (uinfo->value.enumerated.item > e->max - 1) uinfo->value.enumerated.item = e->max - 1; - strcpy(uinfo->value.enumerated.name, - e->texts[uinfo->value.enumerated.item]); + strncpy(uinfo->value.enumerated.name, + e->texts[uinfo->value.enumerated.item], + SNDRV_CTL_ELEMENT_INFO_NAME_LENGTH); return 0; } EXPORT_SYMBOL_GPL(snd_soc_info_enum_double); -- 1.5.5.2
On Tue, Jan 18, 2011 at 7:19 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Tue, Jan 18, 2011 at 05:35:22PM +0000, Neil Jones wrote:
tansen_input_select[] array of strings which only has 3 entries, so it blows up (e->max = 7??).
This...
also shouldn't the strcpy by a strncpy as we are copying to a fixed 64byte buffer ?
There's rather a lot of strcpy() calls in the kernel; perhaps you could be more specific? Ideally if you think there's a change that should be made please post a patch.
static const struct soc_enum tansen_ip_enum = SOC_ENUM_SINGLE(TANSEN_TM_IP_TOP2, 4, 0x7, tansen_input_select);
...is exactly what you told the driver to do. You've told the core there are seven items in the enumeration so it's expecting an array of seven strings. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Thu, Jan 20, 2011 at 04:12:12PM +0000, Neil Jones wrote:
...is exactly what you told the driver to do. You've told the core there are seven items in the enumeration so it's expecting an array of seven strings.
DUH! sorry, in my head i read .xmax as .xmask cheers for the spot.
I still get 'Failed to add route Line Input->Input Mux' though ?
You've probably typoed one of the strings. Look at the code to see why it's detecting this error.
@@ -761,6 +761,8 @@ typedef int __bitwise snd_ctl_elem_iface_t; #define SNDRV_CTL_POWER_D3hot (SNDRV_CTL_POWER_D3|0x0000) /* Off, with power */ #define SNDRV_CTL_POWER_D3cold (SNDRV_CTL_POWER_D3|0x0001) /* Off, without power */
+#define SNDRV_CTL_ELEMENT_INFO_NAME_LENGTH 64
struct snd_ctl_elem_id { unsigned int numid; /* numeric identifier, zero = invalid */ snd_ctl_elem_iface_t iface; /* interface identifier */ @@ -799,7 +801,8 @@ struct snd_ctl_elem_info { struct { unsigned int items; /* R: number of items */ unsigned int item; /* W: item number */
char name[64]; /* R: value name */
/* R: value name */char name[SNDRV_CTL_ELEMENT_INFO_NAME_LENGTH]; } enumerated; unsigned char reserved[128]; } value;
This I'd submit separately to Takashi.
--- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2056,8 +2056,9 @@ int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
if (uinfo->value.enumerated.item > e->max - 1) uinfo->value.enumerated.item = e->max - 1;
strcpy(uinfo->value.enumerated.name,e->texts[uinfo->value.enumerated.item]);
strncpy(uinfo->value.enumerated.name,e->texts[uinfo->value.enumerated.item],SNDRV_CTL_ELEMENT_INFO_NAME_LENGTH); return 0;
ARRAY_SIZE() would do just as well here but this does look reasonable.
participants (2)
-
Mark Brown -
Neil Jones