Hi,
On Thu, Sep 29, 2022 at 04:37:54PM +0200, Jaroslav Kysela wrote:
> The driver field in the struct snd_ctl_card_info is a valid
> user space identifier. Actually, many ASoC drivers do not care
> and let to initialize this field using a standard wrapping method.
> Unfortunately, in this way, this field becomes unusable and
> unreadable for the drivers with longer card names. Also,
> there is a possibility to have clashes (driver field has
> only limit of 15 characters).
>
> This change will print an error when the wrapping is used.
> The developers of the affected drivers should fix the problem.
>
> Signed-off-by: Jaroslav Kysela <perex@xxxxxxxx>
>
> v1..v2:
> - remove the wrong DMI condition per Mark's review
> ---
> sound/soc/soc-core.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index e824ff1a9fc0..590143ebf6df 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1840,21 +1840,22 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
> }
> }
>
> -#define soc_setup_card_name(name, name1, name2, norm) \
> - __soc_setup_card_name(name, sizeof(name), name1, name2, norm)
> -static void __soc_setup_card_name(char *name, int len,
> - const char *name1, const char *name2,
> - int normalization)
> +#define soc_setup_card_name(card, name, name1, name2) \
> + __soc_setup_card_name(card, name, sizeof(name), name1, name2)
> +static void __soc_setup_card_name(struct snd_soc_card *card,
> + char *name, int len,
> + const char *name1, const char *name2)
> {
> + const char *src = name1 ? name1 : name2;
> int i;
>
> - snprintf(name, len, "%s", name1 ? name1 : name2);
> + snprintf(name, len, "%s", src);
>
> - if (!normalization)
> + if (name != card->snd_card->driver)
> return;
The changes do more than the commit message implies by also reworking the need
for the normalization flag, and now that you're special casing most of the
function based on a particular pointer, I wonder if splitting the below logic to
a separate function would make more sense.
In any case,
Reviewed-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>
Thanks,
Nícolas
>
> /*
> - * Name normalization
> + * Name normalization (driver field)
> *
> * The driver name is somewhat special, as it's used as a key for
> * searches in the user-space.
> @@ -1874,6 +1875,14 @@ static void __soc_setup_card_name(char *name, int len,
> break;
> }
> }
> +
> + /*
> + * The driver field should contain a valid string from the user view.
> + * The wrapping usually does not work so well here. Set a smaller string
> + * in the specific ASoC driver.
> + */
> + if (strlen(src) > len - 1)
> + dev_err(card->dev, "ASoC: driver name too long '%s' -> '%s'\n", src, name);
> }
>
> static void soc_cleanup_card_resources(struct snd_soc_card *card)
> @@ -2041,12 +2050,12 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
> /* try to set some sane longname if DMI is available */
> snd_soc_set_dmi_name(card, NULL);
>
> - soc_setup_card_name(card->snd_card->shortname,
> - card->name, NULL, 0);
> - soc_setup_card_name(card->snd_card->longname,
> - card->long_name, card->name, 0);
> - soc_setup_card_name(card->snd_card->driver,
> - card->driver_name, card->name, 1);
> + soc_setup_card_name(card, card->snd_card->shortname,
> + card->name, NULL);
> + soc_setup_card_name(card, card->snd_card->longname,
> + card->long_name, card->name);
> + soc_setup_card_name(card, card->snd_card->driver,
> + card->driver_name, card->name);
>
> if (card->components) {
> /* the current implementation of snd_component_add() accepts */
> --
> 2.35.3
>
