Re: [PATCH v2] usb: gadget: f_uac1_legacy: remove broken string configfs attributes

[Frank Li <Frank.li@oss.nxp.com>]

On Mon, Jul 13, 2026 at 02:08:45PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
>
> The UAC1_STR_ATTRIBUTE macro defines configfs show/store handlers for
> the fn_play, fn_cap, and fn_cntl string options. The store function
> contains an inverted null check on the kstrndup() return value.
>
> This means every write attempt returns -ENOMEM on success and
> dereferences a NULL pointer on allocation failure. The attributes
> have been broken and unused for many years.
>
> Remove the UAC1_STR_ATTRIBUTE macro and the three attributes it
> generated. The internal defaults (FILE_PCM_PLAYBACK, FILE_PCM_CAPTURE,
> FILE_CONTROL) set in f_audio_alloc_inst() are unaffected.
>
> Fixes: 0854611a19ae ("usb: gadget: f_uac1: add configfs support")
> Link: https://lore.kernel.org/linux-usb/20260625113154.1954813-1-xu.yang_2@oss.nxp.com/
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Assisted-by: Claude:claude-sonnet-4.6
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> Changes in v2:
>  - Remove fn_play, fn_cap, and fn_cntl in gadget-testing.rst
>  - Remove fn_*_alloc variable
> ---
>  .../testing/configfs-usb-gadget-uac1_legacy   |  3 -
>  Documentation/usb/gadget-testing.rst          |  3 -
>  drivers/usb/gadget/function/f_uac1_legacy.c   | 56 -------------------
>  drivers/usb/gadget/function/u_uac1_legacy.h   |  3 -
>  4 files changed, 65 deletions(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy b/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy
> index b2eaefd9bc49..6a681d219f43 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy
> @@ -5,8 +5,5 @@ Description:
>  		The attributes:
>
>  		audio_buf_size - audio buffer size
> -		fn_cap - capture pcm device file name
> -		fn_cntl - control device file name
> -		fn_play - playback pcm device file name
>  		req_buf_size - ISO OUT endpoint request buffer size
>  		req_count - ISO OUT endpoint request count
> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> index a6e8292f320a..4921e5307d49 100644
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -714,9 +714,6 @@ The uac1 function provides these attributes in its function directory:
>
>  	=============== ====================================
>  	audio_buf_size	audio buffer size
> -	fn_cap		capture pcm device file name
> -	fn_cntl		control device file name
> -	fn_play		playback pcm device file name
>  	req_buf_size	ISO OUT endpoint request buffer size
>  	req_count	ISO OUT endpoint request count
>  	=============== ====================================
> diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
> index 5d201a2e30e7..3f52099a4fdd 100644
> --- a/drivers/usb/gadget/function/f_uac1_legacy.c
> +++ b/drivers/usb/gadget/function/f_uac1_legacy.c
> @@ -888,60 +888,10 @@ UAC1_INT_ATTRIBUTE(req_buf_size);
>  UAC1_INT_ATTRIBUTE(req_count);
>  UAC1_INT_ATTRIBUTE(audio_buf_size);
>
> -#define UAC1_STR_ATTRIBUTE(name)					\
> -static ssize_t f_uac1_opts_##name##_show(struct config_item *item,	\
> -					 char *page)			\
> -{									\
> -	struct f_uac1_legacy_opts *opts = to_f_uac1_opts(item);		\
> -	int result;							\
> -									\
> -	mutex_lock(&opts->lock);					\
> -	result = sprintf(page, "%s\n", opts->name);			\
> -	mutex_unlock(&opts->lock);					\
> -									\
> -	return result;							\
> -}									\
> -									\
> -static ssize_t f_uac1_opts_##name##_store(struct config_item *item,	\
> -					  const char *page, size_t len)	\
> -{									\
> -	struct f_uac1_legacy_opts *opts = to_f_uac1_opts(item);		\
> -	int ret = -EBUSY;						\
> -	char *tmp;							\
> -									\
> -	mutex_lock(&opts->lock);					\
> -	if (opts->refcnt)						\
> -		goto end;						\
> -									\
> -	tmp = kstrndup(page, len, GFP_KERNEL);				\
> -	if (tmp) {							\
> -		ret = -ENOMEM;						\
> -		goto end;						\
> -	}								\
> -	if (opts->name##_alloc)						\
> -		kfree(opts->name);					\
> -	opts->name##_alloc = true;					\
> -	opts->name = tmp;						\
> -	ret = len;							\
> -									\
> -end:									\
> -	mutex_unlock(&opts->lock);					\
> -	return ret;							\
> -}									\
> -									\
> -CONFIGFS_ATTR(f_uac1_opts_, name)
> -
> -UAC1_STR_ATTRIBUTE(fn_play);
> -UAC1_STR_ATTRIBUTE(fn_cap);
> -UAC1_STR_ATTRIBUTE(fn_cntl);
> -
>  static struct configfs_attribute *f_uac1_attrs[] = {
>  	&f_uac1_opts_attr_req_buf_size,
>  	&f_uac1_opts_attr_req_count,
>  	&f_uac1_opts_attr_audio_buf_size,
> -	&f_uac1_opts_attr_fn_play,
> -	&f_uac1_opts_attr_fn_cap,
> -	&f_uac1_opts_attr_fn_cntl,
>  	NULL,
>  };
>
> @@ -956,12 +906,6 @@ static void f_audio_free_inst(struct usb_function_instance *f)
>  	struct f_uac1_legacy_opts *opts;
>
>  	opts = container_of(f, struct f_uac1_legacy_opts, func_inst);
> -	if (opts->fn_play_alloc)
> -		kfree(opts->fn_play);
> -	if (opts->fn_cap_alloc)
> -		kfree(opts->fn_cap);
> -	if (opts->fn_cntl_alloc)
> -		kfree(opts->fn_cntl);
>  	kfree(opts);
>  }
>
> diff --git a/drivers/usb/gadget/function/u_uac1_legacy.h b/drivers/usb/gadget/function/u_uac1_legacy.h
> index b5df9bcbbeba..b9ddae550ff3 100644
> --- a/drivers/usb/gadget/function/u_uac1_legacy.h
> +++ b/drivers/usb/gadget/function/u_uac1_legacy.h
> @@ -62,9 +62,6 @@ struct f_uac1_legacy_opts {
>  	char				*fn_cap;
>  	char				*fn_cntl;
>  	unsigned			bound:1;
> -	unsigned			fn_play_alloc:1;
> -	unsigned			fn_cap_alloc:1;
> -	unsigned			fn_cntl_alloc:1;
>  	struct mutex			lock;
>  	int				refcnt;
>  };
> --
> 2.34.1
>
>