[alsa-devel] [PATCH 2/2] ALSA: snd-usb: use strncpy() in mixer_quirks.c
Daniel Mack
daniel at zonque.org
Mon Oct 20 16:47:25 CEST 2014
On 10/20/2014 04:40 PM, Takashi Iwai wrote:
> At Sun, 19 Oct 2014 11:38:58 +0200,
> Takashi Iwai wrote:
>>
>> At Sun, 19 Oct 2014 09:11:26 +0200,
>> Daniel Mack wrote:
>>>
>>> Out of principles, use strncpy() in favor of strcpy().
>>> That is, however, an insignificant detail here.
>>>
>>> Signed-off-by: Daniel Mack <daniel at zonque.org>
>>
>> Well, blindly doing this isn't optimal, IMO.
>> First off, strlcpy() is a better one. And, in the code you patched,
>> we already know all strings to be passed. That is, if anything is
>> over the buffer size, it's a clear bug. This can be caught by static
>> analyzers, or put some debug codes (either for build time or compile
>> time) instead of silently trimming the string.
>
> BTW, there is already a nice helper function, snd_ctl_enum_info(), for
> the safe enum info setup. Then the patch would become even more
> reducing, something like below. We can cover many other places in
> similar ways.
>
> A further step would be to add a kernel warning when the given string
> is too long as an enum item string. Then we can catch the buggy
> driver, too. I'll cook up the patch.
Sounds good to me. And yes, we know which values we write to that buffer
and that they can't possibly explode it. I was just thinking of the next
reader of the code who might be quicker in the review with some obvious
guards in place.
Thanks,
Daniel
>
>
> Takashi
>
> ---
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index f119a41ed9a9..dd4d5bdea423 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -441,15 +441,7 @@ static int snd_emu0204_ch_switch_info(struct snd_kcontrol *kcontrol,
> "3/4"
> };
>
> - uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
> - uinfo->count = 1;
> - uinfo->value.enumerated.items = 2;
> - if (uinfo->value.enumerated.item > 1)
> - uinfo->value.enumerated.item = 1;
> - strcpy(uinfo->value.enumerated.name,
> - texts[uinfo->value.enumerated.item]);
> -
> - return 0;
> + return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
> }
>
> static int snd_emu0204_ch_switch_get(struct snd_kcontrol *kcontrol,
> @@ -745,15 +737,7 @@ static int snd_ftu_eff_switch_info(struct snd_kcontrol *kcontrol,
> "Echo"
> };
>
> - uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
> - uinfo->count = 1;
> - uinfo->value.enumerated.items = 8;
> - if (uinfo->value.enumerated.item > 7)
> - uinfo->value.enumerated.item = 7;
> - strcpy(uinfo->value.enumerated.name,
> - texts[uinfo->value.enumerated.item]);
> -
> - return 0;
> + return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(texts), texts);
> }
>
> static int snd_ftu_eff_switch_get(struct snd_kcontrol *kctl,
>
More information about the Alsa-devel
mailing list