| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87a52gb9cr.wl-tiwai@suse.de>
Date: Sat, 27 Sep 2025 10:14:28 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Victor Krawiec <victor.krawiec@...uria.com>
Cc: gregkh@...uxfoundation.org,
linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org,
tiwai@...e.de,
kees@...nel.org,
abdul.rahim@...ahoo.com,
jilliandonahue58@...il.com,
jkeeping@...usicbrands.com
Subject: Re: [PATCH] usb: gadget: f_midi: allow customizing the USB MIDI interface string through configfs
On Wed, 24 Sep 2025 17:48:21 +0200,
Victor Krawiec wrote:
>
> When using f_midi from configfs the USB MIDI interface string is hardcoded
> to 'MIDI function'.
>
> This USB string descriptor is used by some third-party OS or software to
> display the name of the MIDI device
>
> Since we add an additional string option a new macro block was created to
> factorize declarations
>
> Signed-off-by: Victor Krawiec <victor.krawiec@...uria.com>
> ---
> drivers/usb/gadget/function/f_midi.c | 108 +++++++++++++++------------
> drivers/usb/gadget/function/u_midi.h | 8 +-
> 2 files changed, 66 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index da82598fcef8..0a8af7d507d9 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -875,6 +875,7 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
> struct usb_composite_dev *cdev = c->cdev;
> struct f_midi *midi = func_to_midi(f);
> struct usb_string *us;
> + struct f_midi_opts *opts;
> int status, n, jack = 1, i = 0, endpoint_descriptor_index = 0;
>
> midi->gadget = cdev->gadget;
> @@ -883,6 +884,10 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
> if (status < 0)
> goto fail_register;
>
> + opts = container_of(f->fi, struct f_midi_opts, func_inst);
> + if (opts->interface_string_allocated && opts->interface_string)
> + midi_string_defs[STRING_FUNC_IDX].s = opts->interface_string;
> +
> /* maybe allocate device-global string ID */
> us = usb_gstrings_attach(c->cdev, midi_strings,
> ARRAY_SIZE(midi_string_defs));
> @@ -1178,59 +1183,62 @@ end: \
> \
> CONFIGFS_ATTR(f_midi_opts_, name);
>
> +#define F_MIDI_OPT_STRING(name) \
> +static ssize_t f_midi_opts_##name##_show(struct config_item *item, char *page) \
> +{ \
> + struct f_midi_opts *opts = to_f_midi_opts(item); \
> + ssize_t result; \
> + \
> + mutex_lock(&opts->lock); \
> + if (opts->name) { \
> + result = strscpy(page, opts->name, PAGE_SIZE); \
> + } else { \
> + page[0] = 0; \
> + result = 0; \
> + } \
> + \
> + mutex_unlock(&opts->lock); \
> + \
> + return result; \
> +} \
> + \
> +static ssize_t f_midi_opts_##name##_store(struct config_item *item, \
> + const char *page, size_t len) \
> +{ \
> + struct f_midi_opts *opts = to_f_midi_opts(item); \
> + int ret; \
> + char *c; \
> + \
> + mutex_lock(&opts->lock); \
> + if (opts->refcnt > 1) { \
> + ret = -EBUSY; \
> + goto end; \
> + } \
> + \
> + c = kstrndup(page, len, GFP_KERNEL); \
> + if (!c) { \
> + ret = -ENOMEM; \
> + goto end; \
> + } \
> + if (opts->name##_allocated) \
> + kfree(opts->name); \
> + opts->name = c; \
> + opts->name##_allocated = true; \
> + ret = len; \
> +end: \
> + mutex_unlock(&opts->lock); \
> + return ret; \
> +} \
> + \
> +CONFIGFS_ATTR(f_midi_opts_, name);
> +
> F_MIDI_OPT_SIGNED(index, true, SNDRV_CARDS);
> F_MIDI_OPT(buflen, false, 0);
> F_MIDI_OPT(qlen, false, 0);
> F_MIDI_OPT(in_ports, true, MAX_PORTS);
> F_MIDI_OPT(out_ports, true, MAX_PORTS);
> -
> -static ssize_t f_midi_opts_id_show(struct config_item *item, char *page)
> -{
> - struct f_midi_opts *opts = to_f_midi_opts(item);
> - ssize_t result;
> -
> - mutex_lock(&opts->lock);
> - if (opts->id) {
> - result = strscpy(page, opts->id, PAGE_SIZE);
> - } else {
> - page[0] = 0;
> - result = 0;
> - }
> -
> - mutex_unlock(&opts->lock);
> -
> - return result;
> -}
> -
> -static ssize_t f_midi_opts_id_store(struct config_item *item,
> - const char *page, size_t len)
> -{
> - struct f_midi_opts *opts = to_f_midi_opts(item);
> - int ret;
> - char *c;
> -
> - mutex_lock(&opts->lock);
> - if (opts->refcnt > 1) {
> - ret = -EBUSY;
> - goto end;
> - }
> -
> - c = kstrndup(page, len, GFP_KERNEL);
> - if (!c) {
> - ret = -ENOMEM;
> - goto end;
> - }
> - if (opts->id_allocated)
> - kfree(opts->id);
> - opts->id = c;
> - opts->id_allocated = true;
> - ret = len;
> -end:
> - mutex_unlock(&opts->lock);
> - return ret;
> -}
> -
> -CONFIGFS_ATTR(f_midi_opts_, id);
> +F_MIDI_OPT_STRING(id);
> +F_MIDI_OPT_STRING(interface_string);
>
> static struct configfs_attribute *midi_attrs[] = {
> &f_midi_opts_attr_index,
> @@ -1239,6 +1247,7 @@ static struct configfs_attribute *midi_attrs[] = {
> &f_midi_opts_attr_in_ports,
> &f_midi_opts_attr_out_ports,
> &f_midi_opts_attr_id,
> + &f_midi_opts_attr_interface_string,
> NULL,
> };
>
> @@ -1264,6 +1273,8 @@ static void f_midi_free_inst(struct usb_function_instance *f)
> if (free) {
> if (opts->id_allocated)
> kfree(opts->id);
> + if (opts->interface_string_allocated)
> + kfree(opts->interface_string);
> kfree(opts);
> }
> }
> @@ -1280,6 +1291,7 @@ static struct usb_function_instance *f_midi_alloc_inst(void)
> opts->func_inst.free_func_inst = f_midi_free_inst;
> opts->index = SNDRV_DEFAULT_IDX1;
> opts->id = SNDRV_DEFAULT_STR1;
> + opts->interface_string = SNDRV_DEFAULT_STR1;
SNDRV_DEFAULT_STR1 is NULL, and this fact can simplify things a lot.
So, it's better to initialize explicitly with NULL instead of
SNDRV_DEFAULT_STR1, then you can just do NULL-check of the pointer
without the use of *_allocated field (that can be dropped).
Other than that, the code change looks good.
thanks,
Takashi
Powered by blists - more mailing lists